GNOME Bugzilla – Bug 609015
changes for "more apps" view
Last modified: 2010-03-31 15:31:16 UTC
So at a high level we'd really like "applications" to be represented in the shell in a consistent way. This means they should (as much as possible) look and behave similarly every where the concept is used. So far this is: * app well * search results * "more apps" view * to some extent alt+tab * to some extent the app menu in the top bar For the "more apps" part we've proposed doing something simple like: http://www.gnome.org/~mccann/shell/mockups/20091114/shell-mockup-overview-moreapps.png Which will hopefully make the view very similar to the app well. The hope is that this will reinforce the idea that they are related and behave the same way. And in fact things can be moved between them. We have an opportunity to use a fairly wide area of the screen to show the apps. This is good. I'd also like to take advantage of spatial memory and to allow users to reposition app objects in the space. This means that the layout can't be that dynamic. The combination of these desires offers a few challenges. How do you keep a consistent and reliable layout when the screen size changes dynamically? I think we can achieve most of this by not using 100% of the screen width, but a reasonable minimum size - I don't know - say 800px? And then allowing vertical scrolling if necessary. A side effect of not taking the entire screen is that we'd still retain at least the possibility of being able to drag and drop apps onto workspaces. By default we probably want to try to lay out applications in order of "interestingness." And position utility type applications toward the end of the space. At one point we considered sequestering them in a folder or something but I'm not sure this extra layer is necessary. In the mockup we show an alphabetical ordering but I'm not sure that is very interesting. Application icons may be dragged to reorder them. Also they may be dragged into the app well to create favorites. Or onto workspaces to use them. As application icons are moved around places for them to drop should appear below them after a moment. This is important because the insertion point can be very hard to know otherwise. One thing the mockup does not do is integrate very nicely with the "arrow". I think we will want to develop this part further.
We will need to be much more sophisticated with the drag and drop behaviors. Jon touches on this, and I will try to be a little more specific. - when the user clicks and holds and begins to move with the icon, it should start the drag event. This generally works now, but seems a little flaky. - The cursor position on the icon while dragging should match the position at the start of the drag. So if you start dragging from the center of the icon, then during the drag event, the icon should always be on the center of the dragged icon. Currently this is imprecise and there is a lag while dragging. - When an icon is being dragged, we should treat it and the original placeholder visually. Some cases: -- Dragging an icon that is a favorite. When you start dragging, the icon is now attached to the cursor, semitransparent. In the spot where it started should be a darkened icon. This darkened icon and space remains until the next event. (a)the user hovers for a moment over another favorite well position, in which case the previous position is collapsed and the new position animates open as the icons shift out of the way to accommodate the new space, if needed. The new drop position could be marked with the regular hover state outline. There should be only a slight delay in activating a new drop zone. (b)In the event that the user drags the icon over a workspace, the darkened icon should remain until the user lets go. At this point the icon brightens to normal and the app is launched in the corresponding workspace. When the user is holding the icon over a launch activation zone, the cursor should add a special indicator to convey that the launch action will occur. (c)If the user cancels the drag operation by dropping the icon somewhere where there is no activation drop zone, The cursor-attached icon should disappear, and the original icon should brighten to normal. Note on this - we might want to specify in these cases that this means to remove the favorite. If we go this route we need an indicator on the cursor so the user knows dropping here means remove. -- Dragging an icon from the apps list is similar to the above. The only exception may be that if we do not allow users to re-order the apps list, there will be no re-order behavior. Also, dragging to the favorites well or to a workspace should not cause the original spot in the apps list to collapse, since this is really a copy, not a move.
More thoughts on the apps list On the ordering of the apps list. I'm personally ok with abc order as a default, though I admit it is arbitrary. We could add a simple widget at the top of the list that lets users toggle a few different order styles, which could include labeled sections in some cases. Some possible orders might be - type (some scheme that we define like mccann suggested) - category (legacy, if we want to keep this around) - recent - frequent IMHO, Its less important to have ways to order or sort the apps list, since we want to promote the use of favorites frequently used apps. I do see the value though, and I guess gnome users might be accustomed to using a larger # of apps than users on other OS desktops. For the apps list itself, you get cases where apps are cut off partially as you scroll up and down. It would be nice if there was a bit of a fade-out to transparent on these top and bottom edges of the scrollable area.
Jeremy has done a sketch of some of these ideas here: http://img.skitch.com/20100205-d9cuxec8jfc9i7nm16nqgffdwi.png I really like the fade out at the bottom and the use of the scrollbar looks pretty good. I also like the treatment for the application header and arrow. That reinforces the idea that they are connected and shows that it is one way to close the view. I do like having the idea of using different forms of sorting in mind when implementing this but I don't think I want to see this in the first cut. I'd like to try it with the simple layout case initially. So, we should try to align the app icons as they appear in the secondary "more" view with the rows in the app well. Hopefully, at this point we have enough to give it a try.
(In reply to comment #3) > Jeremy has done a sketch of some of these ideas here: > http://img.skitch.com/20100205-d9cuxec8jfc9i7nm16nqgffdwi.png > > I really like the fade out at the bottom and the use of the scrollbar looks > pretty good. I also like the treatment for the application header and arrow. > That reinforces the idea that they are connected and shows that it is one way > to close the view. Total bike-shed: why is it so short?
I think he just took my previous mockup and modified it with photoshop is the most likely explanation :) But I think we might want to try to keep it sorta square. That might help with the width issues mentioned in the first comment. I don't really see a problem with it extending vertically the entire height of the "workspace view" or dash area. (ie. not extending into the message area)
Right, somewhat of an artifact of that. I think we *might* want to keep this from being the full height of the workspace overview so that it maintains the feel of a menu. That might be a bogus argument, its just a feeling. Why a menu? because this isn't a place you spend any time - its a picker. You're picking an app to click to launch or drag somewhere.
Another note here on the scrollbar. The visual in the mockup was based on the once from the current svg mockup, but turned 90 degrees. doing that cheaply means the lighting is wrong on the mockup (lighting should be from above, not the side. That asid, we do want to use the same scrollbar style throughout the overview, but the current one needs more refinement. The scollbar can be thinner and subtler, without as strong of an outline on the track. Maybe if the background of this picker has slight transparency as mocked up, the track could be solid black, with only a hint of a lighter outline.
Created attachment 153185 [details] [review] changes for "more apps" view implementation of proposed mockup (It have only two types of view(Name/Type).)
(In reply to comment #8) > Created an attachment (id=153185) [details] [review] > changes for "more apps" view > > implementation of proposed mockup > > (It have only two types of view(Name/Type).) I tested this, it looks good (very close to the mockups!) but I noticed two issues: 1) It shows apps that aren't apps (it seems like it just displays every .desktop file it can find as an app), I have entries like "metacity" or "compiz" with no icons. (they shouldn't be displayed at all). It also displays custom "open with" handlers as apps. 2) When opening an app it should hide the overview and not let it open.
(In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=153185) [details] [review] [details] [review] > > changes for "more apps" view > > > > implementation of proposed mockup > > > > (It have only two types of view(Name/Type).) > > I tested this, it looks good (very close to the mockups!) but I noticed two > issues: > > 1) It shows apps that aren't apps (it seems like it just displays every > .desktop file it can find as an app), I have entries like "metacity" or > "compiz" with no icons. (they shouldn't be displayed at all). > > It also displays custom "open with" handlers as apps. This seems to have nothing to do with your patch, it is present in the old viewer to. (should be filled as a separate bug and fixed).
Cool. A good start. Will record here some comments I made on IRC last night: * I think I'd prefer that we get this working well for the unsorted case first. So, I'd like to remove the "type" mode for now. * The icons themselves should appear and behave identically to those in the app well. So this means square grid, running indicator, same hover prelight, less noticeable grid lines, same menu on right click, etc * The grid in this popup should as much as possible align horizontally with the grid in the app well. * I don't recall if it was there before this patch but the way the workspace view is dimmed out when this box comes up doesn't seem right. It shouldn't make the windows tranparent - just less bright. * It seems like a number of applications are showing up here that shouldn't be. For example, mutter :) My guess is that we aren't honoring the nodisplay keyword. * The scrollbar has some of the issues pointed out by Jeremy above. I guess we need to do a mockup :) * Also, the scrollbar seems to jiggle around when I mouse over it. * Would be nice to try the fade out when items scroll past the edges of the window (as in Jeremy's mockup).
(In reply to comment #11) > * I don't recall if it was there before this patch but the way the workspace > view is dimmed out when this box comes up doesn't seem right. It shouldn't > make the windows tranparent - just less bright. It has nothing to do with the patch, see bug 602466.
Created attachment 153294 [details] [review] changes for "more apps" view Corrected patch. > Would be nice to try the fade out when items scroll past the edges of the > window (as in Jeremy's mockup). It is hard to implement without change StScrollView. The best option is to add this ability directly in StScrollView, is not it?
Getting much nicer. Good work. A few more comments. * I think its fine to do the fade thing as a separate patch * It seems that right click behavior is inconsistent with the app well icons * The rows of the grid in this popup aren't aligned horizontally with the grid rows in the app well. Adding a line of space at the top should fix that. * A little hard to see but at the very bottom of the scroll area the edge moves up and down by a pixel when i scroll. Some rounding effect maybe? * We may want to play with it being a bit wider by default but I'm not sure. We may have to play around and see how it feels.
Created attachment 153304 [details] [review] changes for "more apps" view > * We may want to play with it being a bit wider by default but I'm not sure. > We may have to play around and see how it feels. Now, It get Width from css.
Created attachment 153305 [details] screenshot Here's a screenshot of what I'm seeing with this patch. Notice where the red box is. The grid in the app well doesn't quite align with the one in the more window. Also, the scrollbar should be positioned a little bit farther away from the edge. See this latest mockup for example: http://www.gnome.org/~mccann/screenshots/clips/20100209021318/shell-mockup-overview.png Updating the look of the scrollbars and the color changes should probably happen in another but so ignore those parts :)
Created attachment 153353 [details] [review] changes for "more apps" view
Looks nice. One thing I noticed in quick testing is that it doesn't seem to support dragging icons to the workspace to launch them. We should probably allow that.
Created attachment 153503 [details] [review] changes for "more apps" view Updated patch. Fixed problem with dragging icons to the workspace.
Review of attachment 153503 [details] [review]: ::: data/theme/gnome-shell.css @@ +43,2 @@ } +StScrollBar StBin{ If I understand Owen's comments from https://bugzilla.gnome.org/show_bug.cgi?id=609401#c17 this means every time we create a StBin we need to check the whole hierarchy for a StScrollBar which is expensive. So this one should probably be reworked to use a name. ::: js/ui/appDisplay.js @@ +39,3 @@ + this._apps = []; + this._grid.removeAll(); + _removeAll: function() { Why do we need a this._apps variable? @@ +76,2 @@ /* This class represents a display containing a collection of application items. + * The applications are sorted based on their popularity and etc They're sorted by name now, no? "and etc" is rather non-specific =) @@ +83,3 @@ + this._allItems = []; + _init: function() { +AllAppDisplay.prototype = { Do we still need this _allItems list? The applications are cached in Shell.AppSystem anyways and can always be retrieved from there. @@ +88,3 @@ this._appsStale = true; this._appSystem.connect('installed-changed', Lang.bind(this, function(appSys) { + this._onInstalledChanged(); You should copy what the AppWell is doing for this; the Main.deferredWork infrastructure allows us to avoid recomputing lots of stuff if we're not actually in the overview at the moment. Even more ideally share the code with AppWell. @@ +93,3 @@ + this.actor.visible = false; + this.actor = new St.BoxLayout({ style_class: 'all-app', vertical: true }); + let bin = new St.BoxLayout({ style_class: 'all-app-controls-panel' }); Why are we setting visibility? @@ +140,3 @@ + this._appView.refresh(this._allItems); + this._refreshCache(); + if (this.actor.visible) { If you use deferredWork then you don't need to check actor.visible yourself. @@ +847,1 @@ } It's going to be pretty broken to change the allocation (by setting _height) from inside _allocate; we can't do that. What are you trying to do here? ::: js/ui/overview.js @@ +238,3 @@ // Container to hold popup pane chrome. + this._paneContainer = new St.BoxLayout({ style_class: 'pane' }); Maybe 'overview-pane'? @@ +371,1 @@ // When a pane is displayed, we raise the transparent background to the top Need a space between "{ expand"
Created attachment 153590 [details] [review] changes for "more apps" view Corrected patch > Why do we need a this._apps variable? We need it to prevent garbage collect (App in _addApp).
Created attachment 153885 [details] [review] changes for "more apps" view merge with master
Created attachment 153934 [details] [review] [St] Add support for StScrollPolicy Previously we were hacking out the vertical scrollbar, this patch allow us to sanely say in which directions we want to scroll.
Created attachment 153935 [details] [review] (squashable) Remove hackish scrolling bits
Review of attachment 153934 [details] [review]: Current StScrollView is AUTO for both directions, it seems. See child_adjustment_changed_cb. (I have some concerns about how StScrollable is done - I don't think it's properly integrated with two-pass Clutter layout, and thus is a lot more confusing than it should be. But I haven't really worked through that and let's ignore it for now.) I think you want to go to AUTO/NEVER not ALWAYS_NEVER. Which requires a substantially different implementation - you can't just unconditionally show the scrollbar, you probably want to queue a resize instead.
Created attachment 154064 [details] [review] Add support for StScrollPolicy Previously we were hacking out the vertical scrollbar, this patch allow us to sanely say in which directions we want to scroll.
(In reply to comment #25) > Review of attachment 153934 [details] [review]: > > Current StScrollView is AUTO for both directions, it seems. See > child_adjustment_changed_cb. (I have some concerns about how StScrollable is > done - I don't think it's properly integrated with two-pass Clutter layout, and > thus is a lot more confusing than it should be. But I haven't really worked > through that and let's ignore it for now.) Ok. > I think you want to go to AUTO/NEVER > not ALWAYS_NEVER. Good catch, fixed. > Which requires a substantially different implementation - you > can't just unconditionally show the scrollbar, you probably want to queue a > resize instead. _actor_show will queue a relayout.
Review of attachment 154064 [details] [review]: Hmm, I really think this needs more thorough thinking through about how the ScrollView works - Since this patch doesnt' touch get_preferred_width(), get_preferred_height(), I don't think it can be correct. ::: src/st/st-scroll-view.c @@ +131,3 @@ g_value_get_boolean (value)); + break; + case PROP_HSCROLL_POLICY: These are HSCROLLBAR_POLICY and VSCROLLBAR_POLICY for GtkScrolledWindow, given the ability not to be pointlessly different, it's less mental load for everybody. @@ +506,3 @@ + ST_TYPE_SCROLL_POLICY, + ST_SCROLL_POLICY_AUTOMATIC, + G_PARAM_READWRITE); g_param_spec_enum ("vscrollbar-policy", P_("Vertical Scrollbar Policy"), P_("When the vertical scrollbar is displayed"), GTK_TYPE_POLICY_TYPE, GTK_POLICY_ALWAYS, GTK_PARAM_READABLE | GTK_PARAM_WRITABLE)); @@ +919,3 @@ + + priv->hscroll_policy = hscroll; + priv->vscroll_policy = vscroll; You are missing GObject property notifications (should be at the end of the function, most likely, with a freeze/thaw) @@ +930,3 @@ + else + { + child_hadjustment_notify_cb (G_OBJECT (priv->child), NULL, scroll); Don't understand why you call this. Note this isn't child_adjustment_changed_cb(), which would be the function that hides/shows the scrollbar. @@ +931,3 @@ + { + child_hadjustment_notify_cb (G_OBJECT (priv->child), NULL, scroll); + clutter_actor_show (priv->hscroll); I guess you're argument is that if it was NEVER before and is now AUTO, then clutter_actor_show() will trigger a reallocation, and then child_adjustment_changed_cb() will get called because the values will change and then it will get hidden again if appropriate. That would at least need an explanatory comment, but ugh.
Created attachment 154239 [details] [review] [StScrollView] Add support for GtkPolicyType for scrollbars Previously we were hacking out the vertical scrollbar, this patch allow us to sanely say in which directions we want to scroll. Note this patch intentionally changes St to depend on GTK+ in the API. I believe this is a correct long term change where we should view St as co-evolving with GTK+ rather than replacing or paralleling.
Created attachment 154240 [details] [review] (squashable) Remove hackish scrolling bits
Review of attachment 154239 [details] [review]: Interface looks OK. (Not sure about the GTK+ thing, but we can always adjust later) I'm pretty positive the request/allocation logic is all busted here, if it works for apps-more, good enough for now. ::: src/st/st-scroll-view.c @@ +948,3 @@ + { + child_hadjustment_notify_cb (G_OBJECT (priv->child), NULL, scroll); + clutter_actor_show (priv->hscroll); As noted previously, the Rube Goldberg setup here needs a comment, on the face of this this reads wrong. ::: src/st/st-scroll-view.h @@ +34,2 @@ #include <st/st-bin.h> +#include "st-enum-types.h" You don't need this addition any more ::: tests/interactive/scrolling.js @@ +23,3 @@ +toggle.connect('notify::checked', function () { + v.set_policy(toggle.checked ? St.ScrollPolicy.AUTOMATIC : St.ScrollPolicy.NEVER, St.ScrollPolicy.AUTOMATIC); Can you add a line break here? I think a ternary is confusing once there is other stuff on the same line
Attachment 154239 [details] pushed as 18c5405 - [StScrollView] Add support for GtkPolicyType for scrollbars
Created attachment 154397 [details] [review] Fix incorrect use of Gtk.PolicyType.ALWAYS in St.ScrollView
Created attachment 154398 [details] Screenshot of committed Apps List
(In reply to comment #34) > Created an attachment (id=154398) [details] > Screenshot of committed Apps List I realize that the idea is to somewhat duplicate the style of the Apps area, but everything is definitely very crammed in. It's especially a waste of space on my larger resolution monitor. Worse still, names are often cut off, and there's no longer item descriptions, only more compounded by the fact that there's no tooltips for items in the list. Also note that it not longer matches the expanded Recent Items, which is inconsistent.
(In reply to comment #35) > (In reply to comment #34) > > Created an attachment (id=154398) [details] [details] > > Screenshot of committed Apps List > > I realize that the idea is to somewhat duplicate the style of the Apps area, > but everything is definitely very crammed in. It's especially a waste of space > on my larger resolution monitor. Worse still, names are often cut off, and > there's no longer item descriptions, only more compounded by the fact that > there's no tooltips for items in the list. > > Also note that it not longer matches the expanded Recent Items, which is > inconsistent. Hey Jonathan - Thanks for the feedback! It gets a bit awkward if any one Bugzilla bug gets too long, so we'd really prefer if you have suggestions for how the feature can be improved, that they be provided in a separate bug. (If you have particular specific suggestions, or even better want to make up some mockups - quality doesn't matter - just has to give the general idea - that would be even better :-)
Created attachment 154412 [details] [review] [StScrollView] Fix incorrect assertion The type we don't currently handle is _ALWAYS, my original use of g_return_if_fail was wrong.
Created attachment 154413 [details] [review] [appDisplay] Use AUTOMATIC for apps-more, not ALWAYS No need to display a scrollbar if we don't need to, and ALWAYS isn't yet implemented anyways.
Attachment 154413 [details] pushed as 004cf3d - [appDisplay] Use AUTOMATIC for apps-more, not ALWAYS
*** Bug 601738 has been marked as a duplicate of this bug. ***
Any reason to leave this bug open? Looks pretty fixed to me ...
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #34) > > > Created an attachment (id=154398) [details] [details] [details] > > > Screenshot of committed Apps List > > > > I realize that the idea is to somewhat duplicate the style of the Apps area, > > but everything is definitely very crammed in. It's especially a waste of space > > on my larger resolution monitor. Worse still, names are often cut off, and > > there's no longer item descriptions, only more compounded by the fact that > > there's no tooltips for items in the list. > > > > Also note that it not longer matches the expanded Recent Items, which is > > inconsistent. > > Hey Jonathan - > > Thanks for the feedback! It gets a bit awkward if any one Bugzilla bug gets too > long, so we'd really prefer if you have suggestions for how the feature can be > improved, that they be provided in a separate bug. (If you have particular > specific suggestions, or even better want to make up some mockups - quality > doesn't matter - just has to give the general idea - that would be even better > :-) Filed my thoughts on the changes in this bug with a mockup as bug 610871 https://bugzilla.gnome.org/show_bug.cgi?id=610871
(In reply to comment #41) > Any reason to leave this bug open? Looks pretty fixed to me ... No ...