mike-jumper commented on code in PR #918:
URL: https://github.com/apache/guacamole-client/pull/918#discussion_r1380777986
##########
guacamole/src/main/frontend/src/app/settings/services/preferenceService.js:
##########
@@ -142,6 +142,20 @@ angular.module('settings').provider('preferenceService',
['$injector',
*/
language : getDefaultLanguageKey(),
+ /**
+ * The number of recent connections to display.
+ *
+ * @type Integer
Review Comment:
This should be `@type {number}` (or `@type {!number}` if not nullable).
##########
guacamole/src/main/frontend/src/app/home/controllers/homeController.js:
##########
@@ -59,6 +60,10 @@ angular.module('home').controller('homeController',
['$scope', '$injector',
$scope.filteredConnectionGroupProperties = [
'name'
];
+
+ $scope.showRecentConnections = function() {
Review Comment:
Please document.
##########
guacamole/src/main/frontend/src/app/history/services/guacHistory.js:
##########
@@ -38,7 +39,7 @@ angular.module('history').factory('guacHistory', ['$injector',
* The number of entries to allow before removing old entries based on the
* cutoff.
*/
- var IDEAL_LENGTH = 6;
+ var IDEAL_LENGTH = preferenceService.preferences.numberOfRecentConnections
|| 6;
Review Comment:
Does this have any impact on the behavior of `updateThumbnail()` if the
setting is changed on the fly? I think `updateThumbnail()` might need pull this
value itself, instead of the value being copied within `guacHistory` as a
constant.
##########
guacamole/src/main/frontend/src/app/home/directives/guacRecentConnections.js:
##########
@@ -50,13 +50,24 @@ angular.module('home').directive('guacRecentConnections',
[function guacRecentCo
// Required services
var guacHistory = $injector.get('guacHistory');
+ var preferenceService = $injector.get('preferenceService');
/**
* Array of all known and visible recently-used connections.
*
* @type RecentConnection[]
*/
$scope.recentConnections = [];
+
+ /**
+ * Returns whether or not recent connections should be displayed.
+ *
+ * @returns {Boolean}
+ * true if recent connections should be displayed, otherwise
false.
+ */
+ $scope.showRecentConnections = function showRecentConnections() {
Review Comment:
I think this should be renamed to avoid confusion, as
`showRecentConnections()` sounds like a function that shows the recent
connections section, not a function that returns whether recent connections are
shown.
##########
guacamole/src/main/frontend/src/app/home/directives/guacRecentConnections.js:
##########
@@ -50,13 +50,24 @@ angular.module('home').directive('guacRecentConnections',
[function guacRecentCo
// Required services
var guacHistory = $injector.get('guacHistory');
+ var preferenceService = $injector.get('preferenceService');
/**
* Array of all known and visible recently-used connections.
*
* @type RecentConnection[]
*/
$scope.recentConnections = [];
+
+ /**
+ * Returns whether or not recent connections should be displayed.
+ *
+ * @returns {Boolean}
Review Comment:
This should be `{boolean}` (or `{!boolean}` if this will never be `null`).
##########
guacamole/src/main/frontend/src/app/settings/services/preferenceService.js:
##########
@@ -142,6 +142,20 @@ angular.module('settings').provider('preferenceService',
['$injector',
*/
language : getDefaultLanguageKey(),
+ /**
+ * The number of recent connections to display.
+ *
+ * @type Integer
+ */
+ numberOfRecentConnections: 6,
+
+ /**
+ * Whether or not to show the "Recent Connections" section.
+ *
+ * @type Boolean
Review Comment:
This should be `@type {boolean}` (or `@type {!boolean}` if not nullable).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]