GNOME Bugzilla – Bug 643426
Allow scrolling application categories
Last modified: 2011-02-28 21:58:46 UTC
At least for me, using the scroll wheel would make it much easier to switch among app categories, as I often find myself scrolling, just to see nothing happen, and have to point and click.
Created attachment 182054 [details] [review] AppDisplay: allow changing categories by scrolling over them Respond to scrolling over the app categories by changing them, so it is not required to point and click, or to use the All view.
Comment on attachment 182054 [details] [review] AppDisplay: allow changing categories by scrolling over them Seems ok to me as a hidden UI feature - vertical navigation is pretty closely related to the function of the scroll wheel. Some minor comments on the code. js/ui/appDisplay.js 110 this._view = new AlphabeticalView(); 111 112 this._currentCategory = -1; missing space before 1 - there are spaces around operators even when unary. 132 this._selectCategory(Math.max(this._currentCategory -1, -1)) 133 else if (direction == Clutter.ScrollDirection.DOWN) 134 this._selectCategory(Math.min(this._currentCategory +1, this._sections.length -1)); Missing spaces before numbers. I find this code somewhat hacky, because I think it's really coincidental that the -1 magic flag value for "All" is ordered immediately before the 0..n-1 categories. But since it does work out pretty well, not really worth adding more code. Can you add a coment // The category list starts with all, with a value of -1, followed // by the normal categories, 0, 1, ... Or something like that. 136 137 _selectCategory: function(num) { 138 this._currentCategory = num; Since we are now tracking the current category, why don't we add a short-circuit here: if (this._currentCategory == num) return; instead of doing a bunch of expensive work when, say, the user scrolls at the end.
(In reply to comment #2) > 112 this._currentCategory = -1; > > missing space before 1 - there are spaces around operators even when unary. > > 132 this._selectCategory(Math.max(this._currentCategory -1, -1)) Ignore the 'even when unary part' - apparently that has no general support among GNOME Shell coders :-). But binary operators should as in the second line above should have spaces around them in any case.
Created attachment 182112 [details] [review] AppDisplay: allow changing categories by scrolling over them Respond to scrolling over the app categories by changing them, so it is not required to point and click, or to use the All view.
Review of attachment 182112 [details] [review]: Looks good to commit ::: js/ui/appDisplay.js @@ +113,3 @@ + // is the number of sections + // -2 is a flag to indicate that nothing is selected + // (used only before the actor is mapped the first time) Out of curiousity, why did you add this in this version. (I actually meant the comment to go in _scrollFilter, but OK here) @@ +134,3 @@ + let direction = event.get_scroll_direction(); + if (direction == Clutter.ScrollDirection.UP) + this._selectCategory(Math.max(this._currentCategory - 1, - 1)) Apparently, the second should be '-1' not '- 1' (I looked, and we are consistently using -1 for a numeric -1)
(In reply to comment #5) > Review of attachment 182112 [details] [review]: > > Looks good to commit > > ::: js/ui/appDisplay.js > @@ +113,3 @@ > + // is the number of sections > + // -2 is a flag to indicate that nothing is selected > + // (used only before the actor is mapped the first time) > > Out of curiousity, why did you add this in this version. Because if set to -1, when calling _selectCategory(-1) it would have no effect, even the first time.
Attachment 182112 [details] pushed as 88bcd0a - AppDisplay: allow changing categories by scrolling over them