GNOME Bugzilla – Bug 682050
Hide overview elements while searching
Last modified: 2013-02-14 23:20:17 UTC
Overview elements being the dash, message tray and workspace switcher. They should slide out of view at the start of a search and slide back in if it is cancelled. If the user drags an app launcher, slide the dash and workspace switcher back into view for them to interact with. See https://live.gnome.org/GnomeShell/Design/Whiteboards/Search The following work depends on Bug 681797.
Created attachment 221510 [details] [review] overview and others: Rename item-drag signals to app-drag Since results other than apps should not be draggable, be more descriptive and rename item-drag signals to app-drag signals.
Created attachment 221511 [details] [review] dash: Add show() and hide() methods The this.actor.height > this._maxHeight check is needed because animating the dash out causes constant notify::height signals and so after each one the max height would shrink to the actor height causing the icons to shrink until eventually we get to 16px icons. This way, only increase maxHeight if the actor can be drawn bigger than it is currently set.
Created attachment 221512 [details] [review] workspaceThumbnail: Fix typo, removeThumbmails --> removeThumbnails
Created attachment 221513 [details] [review] workspaceThumbnail: Make ThumbnailsBox track workspace changes itself
Created attachment 221514 [details] [review] workspaceThumbnail: React to scroll event
Created attachment 221515 [details] [review] workspaceThumbnail: Rename show(), hide() and add new methods show() and hide() now slide the thumbnails in and out, respectively. The old show and hide are now _createThumbnails and _destroyThumbnails. We only care about these thumbnails while in the overview, so create them when entering and destroy them on exit.
Created attachment 221516 [details] [review] workspaceThumbnail: Add keyboard nav to ThumbnailsBox
Created attachment 221517 [details] [review] overview: Make dash a public property
Created attachment 221518 [details] [review] overview: Use ThumbnailsBox for standalone workspace thumbnails
Created attachment 221519 [details] [review] workspacesView: Remove handling of 'controls' since ThumbnailsBox is independent This breaks currently functionality of hiding most of the workspace thumbnails when they aren't being used (< 2 workspaces). The workspace thumbnails are always present, or when hidden, their allocation still exists, i.e., the viewSelector does not resize when the thumbnails do. This works similar to the Dash.
Created attachment 221520 [details] [review] viewSelector: Return a new constraint to those that ask Every actor requesting a constraint needs a new one, they can't all share the same one.
Created attachment 221521 [details] [review] viewSelector: Add search-begin signal and connect search signals to hide/show Hide the elements when the search entry is non-empty. Show them if the search is cancelled.
Created attachment 221522 [details] [review] viewSelector: Connect app-drag signals to show/hide overview elements Anytime we begin dragging an app launcher, ensure the overview elements are showing. While searching, if an app-drag ends or is cancelled, hide the overview elements, but don't switch back to windows (which was the old behavior).
Created attachment 221526 [details] [review] messageTray, viewSelector: show/hide message tray on search signals -- Separate commit for now, since message tray is still in flux.
Review of attachment 221510 [details] [review]: Hm. I'm not sure how I feel about this. What if you want to launch a search result on another workspace? Code looks fine, though.
Review of attachment 221511 [details] [review]: Wait, why does animating the x position cause notify::height signals? ::: js/ui/dash.js @@ +305,3 @@ + this.visible = false; + this._hiddenX; + this._targetX; uhhhh @@ +326,3 @@ this._queueRedisplay(); + if (this.actor.height > this._maxHeight) + this._maxHeight = this.actor.height; Separate patch. @@ +503,3 @@ }, + _updateDashX: function() { This is a bit badly named. _calculateDashX or _computeDashX seems better to me. @@ +506,3 @@ + this._targetX = this.actor.get_x(); + + let rtl = (Clutter.get_default_text_direction() == Clutter.TextDirection.RTL); Use this.actor.get_text_direction() @@ +897,3 @@ + this.actor.show(); + Tweener.addTween(this.actor, + return; Indentation seems a bit strange here. @@ +915,3 @@ + onComplete: Lang.bind(this, function () { + this.actor.hide(); + }); Whitespace here needs to be fixed.
Review of attachment 221512 [details] [review]: Hah. Who made that one? You need to fix the call in workspacesView.js, though.
Review of attachment 221513 [details] [review]: Are you going to remove the old code that triggered this, or?
Review of attachment 221514 [details] [review]: Again, there's code somewhere that deals with scroll events on the workspace thumbnails. Are you going to replace it? ::: js/ui/workspaceThumbnail.js @@ +1181,3 @@ + + _onScrollEvent: function (actor, event) { + switch ( event.get_scroll_direction() ) { Whitespace is bad.
Review of attachment 221515 [details] [review]: Same comments for the dash as there are for this code.
Review of attachment 221516 [details] [review]: ::: js/ui/workspaceThumbnail.js @@ +1247,3 @@ + + _onKeyRelease: function (actor, event) { + switch ( event.get_key_symbol() ) { Whitespace. @@ +1255,3 @@ + break; + case Clutter.KEY_Return: + Main.wm.actionMoveWorkspace(Meta.MotionDirection.UP); I'm not so sure about this. I think about in terms of highlighting a workspace, and hitting enter to "select" it. But this is fine too if it works out in practice.
Review of attachment 221517 [details] [review]: Sure.
Review of attachment 221518 [details] [review]: Oh. Oh. You have no idea how happy I am that you are doing this. You are awesome. Again, you need to remove the old code in the view selector I think. I'm also not sure how you're going to handle the zoom fraction stuff where the workspace switcher comes out and takes over part of the view selector. I'd really like it if you went in the direction I suggested, but it might be a bit fancy. ::: js/ui/overview.js @@ +33,2 @@ const DASH_SPLIT_FRACTION = 0.1; +const WORKSPACE_THUMBNAILS_SPLIT_FRACTION = 0.15; I'd like to see these "fractions" (read: lies) replaced at some point. Preferably, I'd like to see the overview become a horizontal StBoxLayout or a custom LayoutManager or something, split between the dash, the view selector, and the workspace thumbnails. Also, you're not joost! Why are you working on this! @@ +516,3 @@ let viewHeight = contentHeight - 2 * this._spacing; let viewY = contentY + this._spacing; + let viewX = rtl ? thumbnailsBoxWidth + this._spacing : dashWidth + this._spacing; Yeah, I'm not so certain this is going in the right direction, but it works for now I guess.
Review of attachment 221519 [details] [review]: Parts of this code should be squashed with previous patches.
Review of attachment 221520 [details] [review]: Sure. It's disappointing that Constraints are tied to an actor, but sure.
Review of attachment 221521 [details] [review]: I'd really like it if the overview itself connected to the signals the view selector spat out, and kept track of its own state. Otherwise, we might have components fighting over the overview's own actors.
Review of attachment 221522 [details] [review]: Ditto for last patch.
Review of attachment 221526 [details] [review]: Why?
If you can attach your incomplete patches here (or push them to a branch somewhere), I can probably clean them up and finish them for 3.6. It would also help with overview relayout.
Comment on attachment 221517 [details] [review] overview: Make dash a public property This is clearly not wrong, however I think a nicer approach is to expose Main.overview.(show|hide)Sidebars() methods and call that from the viewSelector as approriate. That way both dash and workspaceSwitcher can remain private to the overview, and the viewSelector doesn't need to expose any signals, but just call the appropriate method when switching pages. (I know that this patch series predates the mode-switch-kill)
(In reply to comment #23) > Preferably, I'd like to see the overview become a horizontal StBoxLayout or a > custom LayoutManager or something, split between the dash, the view selector, > and the workspace thumbnails. I don't think this is an option - looking at the mockups[0], the viewSelector needs to extend to the right monitor edge (below the workspace switcher, if it is visible). For symmetry reasons (and in preparation for a nicer apps-view animation), it probably makes sense to do the same for the dash. We could still use a vertical BoxLayout that covers the entire area between top bar and message-tray and holds search entry and viewSelector of course ... Also, while we still have categories in the apps view, I think we need to hide the workspace switcher there as well - having the narrow dash on the left and a rather large "side area" of categories and switcher on the right looks very unbalanced to me. Given that we don't support dragging app launchers to categories, I suggest to let the workspace switcher slide in when starting a drag. [0] http://git.gnome.org/browse/gnome-shell-design/plain/mockups/static/search-results.png
A small addition: I was looking at this patch set, because landing it at least partially would probably allow us to not use a private property from the outside in bug 582650. While I really like the changes here, there is quite a bit of work left - as we are in beta now, I think we should focus on fixing up message-tray/lock-screen issues rather than landing this for 3.6. At least it would give us something to look forward to for 3.8 :-)
(In reply to comment #31) > (In reply to comment #23) > > Preferably, I'd like to see the overview become a horizontal StBoxLayout or a > > custom LayoutManager or something, split between the dash, the view selector, > > and the workspace thumbnails. > > I don't think this is an option - looking at the mockups[0], the viewSelector > needs to extend to the right monitor edge (below the workspace switcher, if it > is visible). For symmetry reasons (and in preparation for a nicer apps-view > animation), it probably makes sense to do the same for the dash. I don't see what's impossible about this. During the apps view, we have dash, window switcher, workspace switcher with BoxLayout. When we have search, we hide the dash and the workspace switcher, leaving only the window switcher, which is Stack'd out to become search results, which expands the full length of the screen.
Created attachment 222649 [details] [review] overview and others: Rename item-drag signals to app-drag Since results other than apps should not be draggable, be more descriptive and rename item-drag signals to app-drag signals. -- I'm not convinced it's a superb idea either, but the design asks for it so we can see if we like it in practice.
Created attachment 222650 [details] [review] dash: Add show/hide methods
Created attachment 222651 [details] [review] dash: fix for shrinking dash while animating The this.actor.height > this._maxHeight check is needed because animating the dash out causes constant notify::height signals and so after each one the max height would shrink to the actor height causing the icons to shrink until eventually we get to 16px icons. This way, only increase maxHeight if the actor can be drawn bigger than it is currently set. -- Not sure why it does it either. Could it be due to the Clutter constraint we add to dash.actor in the overview? Changes: * Split from dash hide/show commit
Created attachment 222652 [details] [review] workspaceThumbnail: Fix typo, removeThumbmails --> removeThumbnails -- Changes: * Change it in workspacesDisplay as well.
Created attachment 222653 [details] [review] workspaceThumbnail: Make ThumbnailsBox track workspace changes itself -- Changes: * Squash in removal of duplicated parts in workspacesView.
Created attachment 222654 [details] [review] workspaceThumbnail: React to scroll event -- Changes: * Squash in removal of duplicate parts in workspacesView.
Created attachment 222655 [details] [review] workspaceThumbnail: Rename show(), hide() and add new methods show() and hide() now slide the thumbnails in and out, respectively. The old show and hide are now _createThumbnails and _destroyThumbnails. We only care about these thumbnails while in the overview, so create them when entering and destroy them on exit. -- Changes: * Cleanup * Rename to _computeThumbnailsX
Created attachment 222656 [details] [review] workspaceThumbnail: Add keyboard nav to ThumbnailsBox -- Up/Down moves workspaces and enter leaves the overview (to the workspace you have moved to). I suppose we could switch to where up/down just move which workspace is highlighted (not actually showing its windows in the overview) and enter switches to it (still in the overview), but that doesn't seem too helpful to me.
Created attachment 222657 [details] [review] overview, workspacesView: Use ThumbnailsBox for independent workspace thumbnails Both WorkspacesDisplay and ThumbnailsBox need to know when windows have been restacked. Instead of each tracking changes on their own or trying to call each other, have the overview keep track and do the calculations, emitting a signal with the result. -- Changes: * Keep the dash and thumbnailsBox private. * Make the overview track the 'restacked' signal and emit its own signal for WorkspacesView and ThumbnailsBox to tie into. This is to avoid each tracking 'restacked' on their own and doing the same calculation separately. Not sure I like this approach though. * Squash in removal of duplicate parts in WorkspacesView.
Created attachment 222658 [details] [review] viewSelector: Return a new constraint to those that ask Every actor requesting a constraint needs a new one, they can't all share the same one. -- No changes (besides being rebased, which removed the need for constrainY).
Created attachment 222660 [details] [review] viewSelector, overview: Add search signals and connect them to hide/show Hide the elements when the search entry is non-empty. Show them if the search is cancelled. -- Changes: * Add both search signals to viewSelector (search-begin was removed with mode-kill). * Have the overview connect to the search signals from viewSelector and manage its own elements.
Created attachment 222661 [details] [review] overview: Connect app-drag signals to show/hide overview elements Anytime we begin dragging an app launcher, ensure the overview elements are showing. While searching, if an app-drag ends or is cancelled, hide the overview elements, but don't switch back to windows (which was the old behavior). -- Changes: * Move signal handling and connected hide/show to overview.
Created attachment 222665 [details] [review] messageTray, viewSelector: show/hide message tray on search signals -- The design calls for the message tray to be hidden while searching. I know the new message tray behavior is could still be subject to change so maybe we won't even need this patch if we aren't going to show the message tray by default in the overview (and if they manually call for the message tray to be shown we should probably respect that even while searching). Ideally, having it hidden allows us to reclaim that vertical space for some more results and just helps visually declutter the interface/keeps focus on the results.
Created attachment 222670 [details] [review] viewSelector: this.active --> this.entryNonEmpty 'active' isn't terribly clear about just what is active, this variable is true/false depending on whether or not the search entry has text. -- Maybe others don't agree. Of course, !nonempty is a bit confusing, so we could switch to this.entryEmpty and toggle its state where it is currently used.
Created attachment 222672 [details] [review] viewSelector: remove switching from search results on app-drag-begin -- I'm not sure what our goal behavior is with the changes that came with the mode-kill, but this keeps it how I had it before, with app-drag not triggering showing of workspacesView. This also affects the apps view and as Florian mentions, we probably want to hide the thumbnails when in the apps view and have them come back into view on app-drag. The dash should stay visible so the user can close the apps view by clicking the dash icon. That behavior is not implemented yet.
I'm working on refreshing this patchset to master for 3.8.
Created attachment 235922 [details] [review] workspaceThumbnail: pack ThumbnailsBox in the overview directly
Created attachment 235923 [details] [review] workspaceThumbnail: add methods to show/hide These slide the workspace selector in and out with an animation.
Created attachment 235924 [details] [review] viewSelector: notify on active page changes In order to do this, we also need to move the assignment of this._activePage from the animation onComplete callback to the function itself.
Created attachment 235925 [details] [review] overview: set side controls visibility on view page change For now use the same behavior as before - hide the workspace thumbnails when showing the applications page.
Created attachment 235926 [details] [review] workspace: include the close button overlap in the right side chrome As we do for the top side; the border is overlaied in the top/right corner.
Created attachment 235927 [details] [review] workspacesView: simplify code We always pass the same coordinates to setGeometry and setClipRect in WorkspacesView; remove the latter to simplify code.
Created attachment 235928 [details] [review] dash: add methods to show/hide These slide the dash in and out with an animation.
Created attachment 235929 [details] [review] overview: hide side controls when searching Hide the elements when the search is active. Show them if the search is cancelled.
Created attachment 235930 [details] [review] dash: slide in on drag-begin
Created attachment 235931 [details] [review] workspaceThumbnails: don't slide in for drag if hidden
Created attachment 235932 [details] [review] viewSelector: don't change page on drag begin We now slide in controls as appropriate when a drag is started.
This new patchset doesn't yet touch the message tray. I'd like to look at that behavior separately after this lands.
Review of attachment 235922 [details] [review]: ::: js/ui/workspaceThumbnail.js @@ +496,3 @@ + let child = actor.get_first_child(); + if (child) + child.allocate(box, flags); How is this different from an StBin / ClutterBinLayout? @@ +510,3 @@ + this._content.connect('get-preferred-height', Lang.bind(this, this._getPreferredHeight)); + this._content.connect('allocate', Lang.bind(this, this._allocate)); + this._content._delegate = this; Seems silly we have a custom layout manager and a generic container, but OK.
Review of attachment 235923 [details] [review]: OK.
Review of attachment 235924 [details] [review]: OK.
Review of attachment 235925 [details] [review]: OK.
Review of attachment 235926 [details] [review]: OK.
Review of attachment 235927 [details] [review]: Yes.
Review of attachment 235928 [details] [review]: ::: js/ui/dash.js @@ +367,3 @@ +const DashFixedLayout = new Lang.Class({ + Name: 'DashFixedLayout', + Extends: Clutter.FixedLayout, I think this can be done with a ClutterBinLayout with y_align set to Clutter.ActorAlign.CENTER. @@ +424,3 @@ this._container.add_actor(this._showAppsIcon.actor); + this.actor = new St.Widget({ layout_manager: new DashFixedLayout(), I don't think the layout manager is relevant to sliding -- is it?
Review of attachment 235929 [details] [review]: ::: js/ui/overview.js @@ +277,3 @@ + + let dashVisible = !searchActive; + let thumbnailsVisible = !searchActive && !appsActive; I wonder if this might be clearer in terms of what pages there are. I know the view selector / page system is a bit weird, but maybe: const CurrentView = { WINDOWS: 1, APPS: 2, SEARCH: 3 }; ... let currentView = viewSelector.getCurrentView(); let dashVisible = (currentView == ViewSelector.CurrentView.WINDOWS || currentView == ViewSelector.CurrentView.APPS); let thumbsVisible = (currentView == ViewSelector.CurrentView.WINDOWS); ... You can still do this in terms of three booleans (getWindowsActive, getAppsActive, getSearchActive) if you don't like the enum.
Review of attachment 235930 [details] [review]: I like this.
Review of attachment 235931 [details] [review]: ::: js/ui/workspaceThumbnail.js @@ +601,1 @@ (this._visible && global.screen.n_workspaces > 2); The actor can't have hover either if it's invisible, so it makes sense to do: if (!this._visible || !alwaysZoomOut) or something (I can't see the code paths below)
Review of attachment 235932 [details] [review]: OK.
(In reply to comment #62) > ::: js/ui/workspaceThumbnail.js > @@ +496,3 @@ > + let child = actor.get_first_child(); > + if (child) > + child.allocate(box, flags); > > How is this different from an StBin / ClutterBinLayout? It is, as those will allocate the preferred child size as far as I can see. For how the WorkspaceThumbnails actor currently works, we need to give the full allocatin to the child instead (which doesn't report a sane preferred size). > @@ +510,3 @@ > + this._content.connect('get-preferred-height', Lang.bind(this, > this._getPreferredHeight)); > + this._content.connect('allocate', Lang.bind(this, this._allocate)); > + this._content._delegate = this; > > Seems silly we have a custom layout manager and a generic container, but OK. Yeah, I have no doubt this could be simplified somehow, but I don't feel like spending even more time chasing the perfect actor configuration that gets us to the same result. I'd be glad to work on symplifying this whole layout more after we land this.
(In reply to comment #68) > Review of attachment 235928 [details] [review]: > > ::: js/ui/dash.js > @@ +367,3 @@ > +const DashFixedLayout = new Lang.Class({ > + Name: 'DashFixedLayout', > + Extends: Clutter.FixedLayout, > > I think this can be done with a ClutterBinLayout with y_align set to > Clutter.ActorAlign.CENTER. Won't work, see below. > @@ +424,3 @@ > this._container.add_actor(this._showAppsIcon.actor); > > + this.actor = new St.Widget({ layout_manager: new DashFixedLayout(), > > I don't think the layout manager is relevant to sliding -- is it? It is; we need a fixed layout - we discussed why this was the case in person previously. In a nutshell, if we don't use a fixed layout, the contents will be allocated with keep their position while sliding out, instead of moving with the parent layout. Again, the same rationale from the previous comment applies here as well. I'll attach a new patchset that fixes the other points.
Created attachment 235961 [details] [review] viewSelector: add a method to get the currently active page
Created attachment 235962 [details] [review] overview: set side controls visibility on view page change For now use the same behavior as before - hide the workspace thumbnails when showing the applications page.
Created attachment 235963 [details] [review] overview: hide side controls when searching Hide the elements when the search is active. Show them if the search is cancelled.
Created attachment 235964 [details] [review] dash: add methods to show/hide These slide the dash in and out with an animation.
Created attachment 235965 [details] [review] workspaceThumbnail: add methods to show/hide These slide the workspace selector in and out with an animation.
(In reply to comment #73) > (In reply to comment #62) > > > ::: js/ui/workspaceThumbnail.js > > @@ +496,3 @@ > > + let child = actor.get_first_child(); > > + if (child) > > + child.allocate(box, flags); > > > > How is this different from an StBin / ClutterBinLayout? > > It is, as those will allocate the preferred child size as far as I can see. > For how the WorkspaceThumbnails actor currently works, we need to give the full > allocatin to the child instead (which doesn't report a sane preferred size). http://git.gnome.org/browse/clutter/tree/clutter/clutter-bin-layout.c#n428 It seems it passes it the full allocation. There's a bit of complication related to clutter_actor_allocate_align_full, but setting the actor's x_expand and y_expand to TRUE will make it use the actor's x_align/y_align layout flags, which by default are CLUTTER_ACTOR_ALIGN_FILL, which shouldn't have any issues. Grep around for BinLayout and you should see people using it on actors that have x_expand/y_expand set to TRUE with a comment describing the hack. We do it already.
Created attachment 235983 [details] [review] workspaceThumbnail: pack ThumbnailsBox in the overview directly -- Now using a stock ClutterBinLayout.
Review of attachment 235983 [details] [review]: Much better.
Review of attachment 235961 [details] [review]: ::: js/ui/viewSelector.js @@ +475,3 @@ + + getActivePage: function() { + if (this._activePage == this._workspacesPage) Perhaps later we should store this on the view selector itself instead of using activePage comparisons, but for now this is fine. @@ +476,3 @@ + getActivePage: function() { + if (this._activePage == this._workspacesPage) + return ViewPage.WINDOWS Missing semi.
Review of attachment 235962 [details] [review]: ::: js/ui/overview.js @@ +274,3 @@ + + let activePage = this._viewSelector.getActivePage(); + let thumbnailsVisible = (activePage != ViewSelector.ViewPage.APPS); I think I like (activePage == ViewSelector.ViewPage.WINDOWS || activePage == ViewSelector.ViewPage.SEARCH); better, and then we'll take that second part out later.
Review of attachment 235963 [details] [review]: OK when rebased.
Review of attachment 235965 [details] [review]: OK.
Review of attachment 235964 [details] [review]: If you don't come up with an alternate strategy for this either tonight or tomorrow, feel free to commit. I'd still like to try to replace this with something a bit more standard, though.
Created attachment 236158 [details] [review] Introduce a SlideLayout layout manager SlideLayout is a fixed layout that takes care of requesting and allocating the right sizes so its contents can slide horizontally as the actor is resized. Sliding is controlled with a slideX and slideDirection properties, which do the right thing wrt. RTL automatically.
Created attachment 236161 [details] [review] workspaceThumbnail: pack ThumbnailsBox in the overview directly
Created attachment 236162 [details] [review] overview: set side controls visibility on view page change For now use the same behavior as before - hide the workspace thumbnails when showing the applications page.
Created attachment 236163 [details] [review] dash: add methods to show/hide These slide the dash in and out with an animation.
Review of attachment 236158 [details] [review]: Minor nit, otherwise fine. Nice and clean :) ::: js/ui/slideLayout.js @@ +26,3 @@ + let rtl = (child.text_direction == Clutter.TextDirection.RTL); + if (rtl) + direction = !direction; Not sure I like this. I'd prefer to have: if (rtl) direction = (direction == SlideDirection.LEFT) ? SlideDirection.RIGHT : SlideDirection.LEFT; or something.
Review of attachment 236161 [details] [review]: Yes.
Review of attachment 236163 [details] [review]: I was imagining that the SlideLayout would be in the overview (along with the thumbnails, but that's a bit special). I think it's a bit better to have it there for the dash for now.
Created attachment 236186 [details] [review] Introduce a SlideLayout layout manager SlideLayout is a fixed layout that takes care of requesting and allocating the right sizes so its contents can slide horizontally as the actor is resized. Sliding is controlled with a slideX and slideDirection properties, which do the right thing wrt. RTL automatically. Also add a SlidingControl base class that will be used by the overview to pack and slide the workspace thumbnail switcher and the dash.
Created attachment 236187 [details] [review] workspaceThumbnail: pack ThumbnailsBox in the overview directly Use a SlidingControl subclass and pack that in the overview group directly.
Created attachment 236188 [details] [review] viewSelector: notify on active page changes In order to do this, we also need to move the assignment of this._activePage from the animation onComplete callback to the function itself.
Created attachment 236189 [details] [review] viewSelector: add a method to get the currently active page
Created attachment 236190 [details] [review] overview: set side controls visibility on view page change For now use the same behavior as before - hide the workspace thumbnails when showing the applications page.
Created attachment 236191 [details] [review] workspace: include the close button overlap in the right side chrome As we do for the top side; the border is overlaied in the top/right corner.
Created attachment 236192 [details] [review] workspacesView: simplify code We always pass the same coordinates to setGeometry and setClipRect in WorkspacesView; remove the latter to simplify code.
Created attachment 236193 [details] [review] dash: add a SlidingControl for the dash actor Will be needed to hide and show it when searching.
Created attachment 236194 [details] [review] overview: hide side controls when searching Hide the elements when the search is active. Show them if the search is cancelled.
Created attachment 236195 [details] [review] viewSelector: don't change page on drag begin We now slide in controls as appropriate when a drag is started.
Reattached full patchset (with previous a-c-n statuses) for clarity.
Review of attachment 236186 [details] [review]: Fine to commit with this fixed. ::: js/ui/overviewControls.js @@ +30,3 @@ + let rtl = (child.text_direction == Clutter.TextDirection.RTL); + if (rtl) + direction = !direction; Please fix this.
Review of attachment 236187 [details] [review]: OK.
Review of attachment 236193 [details] [review]: ::: js/ui/overviewControls.js @@ +211,3 @@ + this.dash = dash; + dash.actor.x_expand = true; + dash.actor.y_expand = true; You should add a comment saying that this is the standard BinLayout hack.
All pushed with the suggested changes. Thanks everyone for the reviews, let's wait for the fallout now :) Attachment 236186 [details] pushed as 93bde0c - Introduce a SlideLayout layout manager Attachment 236187 [details] pushed as 3d8a875 - workspaceThumbnail: pack ThumbnailsBox in the overview directly Attachment 236188 [details] pushed as f2edcb9 - viewSelector: notify on active page changes Attachment 236189 [details] pushed as 4016da6 - viewSelector: add a method to get the currently active page Attachment 236190 [details] pushed as 5b39496 - overview: set side controls visibility on view page change Attachment 236191 [details] pushed as bc75e5e - workspace: include the close button overlap in the right side chrome Attachment 236192 [details] pushed as c550e2c - workspacesView: simplify code Attachment 236193 [details] pushed as 2247065 - dash: add a SlidingControl for the dash actor Attachment 236194 [details] pushed as 45415a7 - overview: hide side controls when searching Attachment 236195 [details] pushed as f7512dc - viewSelector: don't change page on drag begin