mike-jumper commented on code in PR #922:
URL: https://github.com/apache/guacamole-client/pull/922#discussion_r1370614602
##########
guacamole/src/main/frontend/src/app/player/directives/player.js:
##########
@@ -179,6 +199,12 @@ angular.module('player').directive('guacPlayer',
['$injector', function guacPlay
*/
var resumeAfterSeekRequest = false;
+ /**
+ * A scheduled timer to clean up the mouse activity CSS class, or null
+ * if no timer is scheduled.
+ */
Review Comment:
Please add `@type` annotation.
##########
guacamole/src/main/frontend/src/app/player/directives/player.js:
##########
@@ -401,6 +427,43 @@ angular.module('player').directive('guacPlayer',
['$injector', function guacPlay
$scope.$on('$destroy', function playerDestroyed() {
$scope.recording.pause();
$scope.recording.abort();
+ mouseActivityTimer !== null &&
$window.clearTimeout(mouseActivityTimer);
+ });
+
+ /**
+ * Clean up the mouse movement class after no mouse activity has been
+ * detected for the appropriate time period.
+ */
+ const scheduleCleanupTimeout = _.debounce(() =>
+ mouseActivityTimer = $window.setTimeout(() => {
+ mouseActivityTimer = null;
+ $element.removeClass('recent-mouse-movement');
+ }, MOUSE_CLEANUP_TIMER_DELAY),
+
+ /*
+ * Only schedule the cleanup task after the mouse hasn't moved
+ * for a reasonable amount of time to ensure that the number of
+ * created cleanup timers remains reasonable.
+ */
+ MOUSE_DEBOUNCE_DELAY);
Review Comment:
> ... to ensure that the number of created cleanup timers remains reasonable.
Is this the case? I see `mouseActivityTimer` as tracking the most recent
scheduled cleanup, and that it's cleared prior to scheduling any future
cleanup. It looks like there will always be at most one registered timeout for
cleanup.
##########
guacamole/src/main/frontend/src/app/player/directives/player.js:
##########
@@ -77,6 +79,24 @@
*/
angular.module('player').directive('guacPlayer', ['$injector', function
guacPlayer($injector) {
+ /**
+ * The number of milliseconds after the last detected mouse activity after
+ * which the associated CSS class should be removed.
+ */
+ const MOUSE_CLEANUP_DELAY = 4000;
+
+ /**
+ * The number of milliseconds after the last detected mouse activity before
+ * the cleanup timer to remove the associated CSS class should be
scheduled.
+ */
+ const MOUSE_DEBOUNCE_DELAY = 250;
+
+ /**
+ * The number of milliseconds, after the debounce delay, before the mouse
+ * activity cleanup timer should run.
+ */
+ const MOUSE_CLEANUP_TIMER_DELAY = MOUSE_CLEANUP_DELAY -
MOUSE_DEBOUNCE_DELAY;
Review Comment:
Please add `@type` annotations.
--
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]
