GNOME Bugzilla – Bug 693987
Don't always show the message tray in the overview
Last modified: 2013-02-17 18:13:44 UTC
Latest design iterations say the message tray should not be specially handled in the overview, and react to the pressure movement and Super+M as normally, giving more space to the central view. Attached patchset implements this.
Created attachment 236408 [details] [review] messageTray: remove overview special handling Don't show the message tray in the overview by default. From now on the message tray in overview behaves as regularly, i.e. it will slide up the overview on Super+M keypress.
Created attachment 236409 [details] [review] overviewControls: account for the search entry in each control Account for the search entry space at the bottom (the former message tray clone) individually in each side control, instead of packing another actor in the overview. This allows us to extend the central view all the way to the bottom, while still keeping controls centered vertically.
Created attachment 236410 [details] [review] overview: export the overview base actor Since this is the one that takes grabs, and we'll need to check if it's holding a grab from the PressureBarrier in a future commit.
Created attachment 236411 [details] [review] main: make _findModal public Rename it to findModalIndex.
Created attachment 236412 [details] [review] pressure-barrier: don't discard grabbed events when overview is grabbing Since we still want to trigger the message tray in that case.
Review of attachment 236412 [details] [review]: ::: js/ui/layout.js @@ +1202,3 @@ + if (event.grabbed && + (!Main.overview.visible || + Main.findModalIndex(Main.overview.actor) != 0)) I'd do this differently. // Throw out all events where the pointer was grabbed // by another client, as the client that grabbed the // pointer expects to have complete control over it. if (event.grabbed && Main.modalCount == 0) return; // Throw out all events where the a non-overview has // grabbed the pointer. if (Main.keybindingMode & (Shell.KeyBindingMode.NORMAL | Shell.KeyBindingMode.OVERVIEW)) return;
Review of attachment 236411 [details] [review]: ::: js/ui/main.js @@ +461,2 @@ function isInModalStack(actor) { + return findModalIndex(actor) != -1; This function seems to be unused, I'd just remove it.
Created attachment 236414 [details] [review] main: remove unused function
Created attachment 236415 [details] [review] pressure-barrier: don't discard grabbed events when overview is grabbing -- Use Main.keybindingMode to find out whether the grab is coming from the overview actor or not.
Review of attachment 236414 [details] [review]: OK.
Review of attachment 236415 [details] [review]: OK.
Review of attachment 236410 [details] [review]: Not needed.
Review of attachment 236408 [details] [review]: OK.
Review of attachment 236415 [details] [review]: ::: js/ui/layout.js @@ +1202,3 @@ + return; + + let isOverview = ((Main.keybindingMode ^ (Shell.KeyBindingMode.OVERVIEW)) == 0); Shell.KeyBindingMode is used as flags when setting up keybingins, but Main.keybindingMode uses it as enum. That means that for instance when the run dialog is open in the overview, isOverview will be false. Why not a simple "Main.overview.visible"?
(In reply to comment #14) > Shell.KeyBindingMode is used as flags when setting up keybingins, but > Main.keybindingMode uses it as enum. That means that for instance when the run > dialog is open in the overview, isOverview will be false. > Why not a simple "Main.overview.visible"? That's what we want; if a modal dialog is showing in the overview, you don't want the pressure to trigger.
(In reply to comment #15) > That's what we want; if a modal dialog is showing in the overview, you don't > want the pressure to trigger. Mmmh, ok then. It still feels like API abuse though ...
(In reply to comment #16) > Mmmh, ok then. It still feels like API abuse though ... Initially I thought the same, but then I realized the pressure trigger is really sort of a keybinding in itself. It makes sense to use the same infrastructure and avoid touching the grab code I think.
Created attachment 236417 [details] [review] overviewControls: account for the search entry in each control -- Clean up some useless properties from constructor.
Review of attachment 236415 [details] [review]: Note that the Shell.KeyBindingMode.OVERVIEW is hardcoded for now, but in the future it may make sense to pass in a keybinding mode. I think we should also do: let isOverview = ((Main.keybindingMode & Shell.KeyBindingMode.OVERVIEW) != 0); instead, to match us closer to what filterKeybinding does.
Review of attachment 236408 [details] [review]: ::: js/ui/messageTray.js @@ +2268,2 @@ // Summary + let summarySummoned = this._pointerInSummary || this._traySummoned; Extra space.
Review of attachment 236417 [details] [review]: ::: js/ui/overviewControls.js @@ +307,3 @@ + + let entryClone = new St.Widget(); + // the entryBin's height never changes Large Text
So, there's a feature where you can hit the hot corner to enter the overview while the message tray is up to enter / exit the overview. I don't know how this would work in the world where we push the overview up. It doesn't work right now because of a few bugs, so if Allan / Jakub don't mind removing support, I can write a patch.
(In reply to comment #22) > So, there's a feature where you can hit the hot corner to enter the overview > while the message tray is up to enter / exit the overview. I don't know how > this would work in the world where we push the overview up. It doesn't work > right now because of a few bugs, so if Allan / Jakub don't mind removing > support, I can write a patch. Ideally, if the tray is up and you hit the hot corner, it would close the tray and the hot corner would react as normal.
Created attachment 236456 [details] [review] overviewControls: account for the search entry in each control -- Update clone height when the search bin height changes - using a ClutterBindConstraint doesn't work for some reason.
Created attachment 236457 [details] [review] pressure-barrier: don't discard grabbed events when overview is grabbing -- Use (Main.keybindingMode & (Shell.KeyBindingMode.OVERVIEW)) != 0 for the check.
(In reply to comment #20) > Review of attachment 236408 [details] [review]: > > ::: js/ui/messageTray.js > @@ +2268,2 @@ > // Summary > + let summarySummoned = this._pointerInSummary || this._traySummoned; > > Extra space. Fixed this locally.
Review of attachment 236457 [details] [review]: OK.
Review of attachment 236456 [details] [review]: Not the happiest person about this, but OK.
Attachment 236408 [details] pushed as 060c049 - messageTray: remove overview special handling Attachment 236414 [details] pushed as ebd7406 - main: remove unused function Attachment 236456 [details] pushed as 66cdbc5 - overviewControls: account for the search entry in each control Attachment 236457 [details] pushed as 53a5958 - pressure-barrier: don't discard grabbed events when overview is grabbing Thanks, pushed to master.