GNOME Bugzilla – Bug 682310
messageTray: add dwelling at the bottom of the screen to open the tray
Last modified: 2012-11-09 12:45:29 UTC
If the user leaves the mouse pointer at the bottom of the screen for a second, open the tray. This simulates the eventual plan of measuring "pressure" by how far the pointer is moved past the edge of the screen. Measuring pressure will take X server changes. Note: the PointerWatcher class here isn't used for the current use of pointer watching in the tray because that uses large (1-4 second timeouts) that closely integrate with the logic.
Created attachment 221931 [details] [review] pointerWatcher: add a class to track the pointer Add a class to efficiently poll the pointer position, stopping polling when the user is idle.
Created attachment 221932 [details] [review] messageTray: add dwelling at the bottom of the screen to open the tray If the user leaves the mouse pointer at the bottom of the screen for a second, open the tray. This simulates the eventual plan of measuring "pressure" by how far the pointer is moved past the edge of the screen. Measuring pressure will take X server changes.
Created attachment 221933 [details] [review] pointerWatcher: add a class to track the pointer [ last set of patches were incorrectly created, reattaching ] Add a class to efficiently poll the pointer position, stopping polling when the user is idle.
Created attachment 221934 [details] [review] messageTray: add dwelling at the bottom of the screen to open the tray If the user leaves the mouse pointer at the bottom of the screen for a second, open the tray. This simulates the eventual plan of measuring "pressure" by how far the pointer is moved past the edge of the screen. Measuring pressure will take X server changes.
Review of attachment 221933 [details] [review]: Good to commit with the fixup to _updateTimeout() ::: js/ui/pointerWatcher.js @@ +54,3 @@ + // not idle, the position of the pointer will be queried at least + // once every this many milliseconds. + // @callback: function to call when the pointer position changes - take takes @@ +88,3 @@ + + _updateTimeout: function() { + let interval = null; let minInterval = this._watches.length > 0 ? this._watches[0].interval : null; would make the purpose of the variable more clear and the for loop simpler @@ +95,3 @@ + interval = this._watches[0].interval; + else + interval = Math.min(this._watches[0].interval, interval); this can start with i = 1 and have minInterval = Math.min(this._watches[i].interval, minInterval);
Review of attachment 221934 [details] [review]: Looks good. Should we remove the hot corner code or do we want the corner to still trigger the tray immediately? Keeping the hot corner keeps the old problems of triggering the tray unintentionally, while adding new ones... ::: js/ui/messageTray.js @@ +1502,3 @@ + _checkTrayDwell: function(x, y) { + // By not even starting when this.actor.hover, we avoid popping up the tray if the + // user mouses down to interact with the tray This would be clearer: We only set up dwell timeout when the user is not hovering over the tray (!this.actor.hover). This avoids bringing up the message tray after the user clicks on a notification they are hovering over with the pointer at the bottom of the monitor. @@ +1504,3 @@ + // user mouses down to interact with the tray + if (y == global.screen_height - 1 && !this.actor.hover) { + this._cancelTrayDwell(); We can keep the old timeout running while the pointer is anywhere along the bottom of the monitor, so no need to cancel the timeout. @@ +1508,3 @@ + this._trayDwellTimeoutId = Mainloop.timeout_add(TRAY_DWELL_TIME, + Lang.bind(this, this._trayDwellTimeout)); + } No curly braces if the conditional is just one statement. @@ +2253,3 @@ if (notification.isTransient) notification.destroy(NotificationDestroyedReason.EXPIRED); + this.actor.hover = false; // Clutter doesn't emit notify::hover when actors move Move it right under this._pointerInTray = false;
Patches pushed, with comments adjusted and _updateTimeout() updated based on your comments. (I did _updateTimeout() a little different to get it even more compact and hopefully readable.) Leaving the bug open for the question of whether we want to remove or modify the hot corner. Attachment 221933 [details] pushed as 4465834 - pointerWatcher: add a class to track the pointer Attachment 221934 [details] pushed as 54f3036 - messageTray: add dwelling at the bottom of the screen to open the tray
Created attachment 222055 [details] [review] Remove the message tray hot corner Since dwell at the bottom of the screen is now the primary way of summoning the tray, removing the hot corner to avoid having two separate things that can be accidentally triggered.
Created attachment 222056 [details] [review] messageTray: Remove the message tray hot corner new version: Remove a left-over reference to the corner variable.
Review of attachment 222056 [details] [review]: Certainly makes sense to me - should we get this in before the release?
Attachment 222056 [details] pushed as 46b7a95 - messageTray: Remove the message tray hot corner
it is easy to change the constant const TRAY_DWELL_TIME in /usr/share/gnome-shell/js/ui/messageTray.js i know. But users should be able to tweak this at least as dconf-constant on per user base. For me 300 ms seems ideal. One second is very long IMO.