After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693987 - Don't always show the message tray in the overview
Don't always show the message tray in the overview
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-16 21:21 UTC by Cosimo Cecchi
Modified: 2013-02-17 18:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
messageTray: remove overview special handling (5.95 KB, patch)
2013-02-16 21:21 UTC, Cosimo Cecchi
committed Details | Review
overviewControls: account for the search entry in each control (4.44 KB, patch)
2013-02-16 21:21 UTC, Cosimo Cecchi
none Details | Review
overview: export the overview base actor (925 bytes, patch)
2013-02-16 21:21 UTC, Cosimo Cecchi
rejected Details | Review
main: make _findModal public (1.92 KB, patch)
2013-02-16 21:21 UTC, Cosimo Cecchi
reviewed Details | Review
pressure-barrier: don't discard grabbed events when overview is grabbing (1.18 KB, patch)
2013-02-16 21:21 UTC, Cosimo Cecchi
reviewed Details | Review
main: remove unused function (659 bytes, patch)
2013-02-16 21:58 UTC, Cosimo Cecchi
committed Details | Review
pressure-barrier: don't discard grabbed events when overview is grabbing (1.42 KB, patch)
2013-02-16 21:58 UTC, Cosimo Cecchi
reviewed Details | Review
overviewControls: account for the search entry in each control (4.40 KB, patch)
2013-02-16 22:31 UTC, Cosimo Cecchi
needs-work Details | Review
overviewControls: account for the search entry in each control (4.51 KB, patch)
2013-02-17 15:22 UTC, Cosimo Cecchi
committed Details | Review
pressure-barrier: don't discard grabbed events when overview is grabbing (1.42 KB, patch)
2013-02-17 15:23 UTC, Cosimo Cecchi
committed Details | Review

Description Cosimo Cecchi 2013-02-16 21:21:04 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.
Comment 1 Cosimo Cecchi 2013-02-16 21:21:07 UTC
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.
Comment 2 Cosimo Cecchi 2013-02-16 21:21:10 UTC
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.
Comment 3 Cosimo Cecchi 2013-02-16 21:21:13 UTC
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.
Comment 4 Cosimo Cecchi 2013-02-16 21:21:16 UTC
Created attachment 236411 [details] [review]
main: make _findModal public

Rename it to findModalIndex.
Comment 5 Cosimo Cecchi 2013-02-16 21:21:19 UTC
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.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-02-16 21:30:48 UTC
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;
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-02-16 21:30:57 UTC
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.
Comment 8 Cosimo Cecchi 2013-02-16 21:58:04 UTC
Created attachment 236414 [details] [review]
main: remove unused function
Comment 9 Cosimo Cecchi 2013-02-16 21:58:38 UTC
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.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-02-16 22:06:08 UTC
Review of attachment 236414 [details] [review]:

OK.
Comment 11 Jasper St. Pierre (not reading bugmail) 2013-02-16 22:06:17 UTC
Review of attachment 236415 [details] [review]:

OK.
Comment 12 Jasper St. Pierre (not reading bugmail) 2013-02-16 22:06:29 UTC
Review of attachment 236410 [details] [review]:

Not needed.
Comment 13 Jasper St. Pierre (not reading bugmail) 2013-02-16 22:06:43 UTC
Review of attachment 236408 [details] [review]:

OK.
Comment 14 Florian Müllner 2013-02-16 22:18:12 UTC
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"?
Comment 15 Cosimo Cecchi 2013-02-16 22:19:46 UTC
(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.
Comment 16 Florian Müllner 2013-02-16 22:22:24 UTC
(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 ...
Comment 17 Cosimo Cecchi 2013-02-16 22:25:21 UTC
(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.
Comment 18 Cosimo Cecchi 2013-02-16 22:31:06 UTC
Created attachment 236417 [details] [review]
overviewControls: account for the search entry in each control

--

Clean up some useless properties from constructor.
Comment 19 Jasper St. Pierre (not reading bugmail) 2013-02-17 04:14:59 UTC
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.
Comment 20 Jasper St. Pierre (not reading bugmail) 2013-02-17 04:33:47 UTC
Review of attachment 236408 [details] [review]:

::: js/ui/messageTray.js
@@ +2268,2 @@
         // Summary
+        let summarySummoned = this._pointerInSummary ||  this._traySummoned;

Extra space.
Comment 21 Jasper St. Pierre (not reading bugmail) 2013-02-17 04:34:11 UTC
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
Comment 22 Jasper St. Pierre (not reading bugmail) 2013-02-17 04:36:46 UTC
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.
Comment 23 Allan Day 2013-02-17 12:55:37 UTC
(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.
Comment 24 Cosimo Cecchi 2013-02-17 15:22:51 UTC
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.
Comment 25 Cosimo Cecchi 2013-02-17 15:23:36 UTC
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.
Comment 26 Cosimo Cecchi 2013-02-17 15:24:18 UTC
(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.
Comment 27 Jasper St. Pierre (not reading bugmail) 2013-02-17 17:56:00 UTC
Review of attachment 236457 [details] [review]:

OK.
Comment 28 Jasper St. Pierre (not reading bugmail) 2013-02-17 17:58:36 UTC
Review of attachment 236456 [details] [review]:

Not the happiest person about this, but OK.
Comment 29 Cosimo Cecchi 2013-02-17 18:13:32 UTC
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.