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 645759 - Clicking on Activities with menu up leaves a funny state
Clicking on Activities with menu up leaves a funny state
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 641253 647282 652591 (view as bug list)
Depends on:
Blocks: 633620
 
 
Reported: 2011-03-26 17:25 UTC by Owen Taylor
Modified: 2011-07-14 19:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
activitiesButton: abstract this out of panel.js (25.81 KB, patch)
2011-05-02 18:35 UTC, Dan Winship
none Details | Review
activitiesButton: make this a PanelMenu.Button (5.67 KB, patch)
2011-05-02 18:35 UTC, Dan Winship
none Details | Review
activitiesButton: make this a PanelMenu.Button (7.93 KB, patch)
2011-06-27 21:07 UTC, Dan Winship
none Details | Review
panel: abstract ActivitiesButton into its own class (8.01 KB, patch)
2011-07-14 15:07 UTC, Dan Winship
committed Details | Review
panel: make ActivitiesButton a PanelMenu.Button (7.62 KB, patch)
2011-07-14 15:07 UTC, Dan Winship
committed Details | Review
panel, layout: clean up HotCorner handling (23.01 KB, patch)
2011-07-14 15:07 UTC, Dan Winship
needs-work Details | Review
panel, layout: clean up HotCorner handling (23.29 KB, patch)
2011-07-14 15:56 UTC, Dan Winship
committed Details | Review

Description Owen Taylor 2011-03-26 17:25:19 UTC
Reported by David Le Sage in https://bugzilla.redhat.com/show_bug.cgi?id=683685

===
If you click on a GNOME Bar/Panel item, such as the Desktop clock and then
(while it is still down) go and click on the Activities area, nothing happens. 
GNOME does not switch to Overlay mode, even if you click it several times.

Steps to Reproduce:
1. Click on the Desktop clock (or any other Panel drop-down, such as the
current app indicator.)
2. With that menu down, go and click on Activities.
===

Reproduced with the latest code in Git.
Comment 1 Jonathan Strander 2011-03-26 22:50:40 UTC
Worth noting that you won't even get the prelight and keyboard nav won't work on it either after a click. But keyboard nav does work properly using Ctrl+Alt+Tab to select the Top Panel.
Comment 2 Jonathan Strander 2011-03-26 22:59:15 UTC
This may be related: I can see Activities flash to it's "activated" state when then popupmenu is dismissed.
Comment 3 Florian Müllner 2011-04-09 16:34:33 UTC
*** Bug 647282 has been marked as a duplicate of this bug. ***
Comment 4 Dan Winship 2011-05-02 18:35:08 UTC
Created attachment 187059 [details] [review]
activitiesButton: abstract this out of panel.js

Move the HotCorner and ActivitiesButton code out of panel.js
Comment 5 Dan Winship 2011-05-02 18:35:11 UTC
Created attachment 187060 [details] [review]
activitiesButton: make this a PanelMenu.Button

The fact that everything in the top bar except the activities button
was a menu made various things difficult. Simplify this by making the
activities button be a menu too, but just hack it up a bit so that the
menu associated with the button never actually appears.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=645759 (Clicking on
Activities with menu up leaves a funny state) and
https://bugzilla.gnome.org/show_bug.cgi?id=641253 (panel keynav
between Activities and menus is quirky).
Comment 6 Dan Winship 2011-06-20 17:20:14 UTC
*** Bug 652591 has been marked as a duplicate of this bug. ***
Comment 7 Dan Winship 2011-06-27 21:07:32 UTC
Created attachment 190814 [details] [review]
activitiesButton: make this a PanelMenu.Button

====

updated, improved a bit.

There is one oddity, which is that if the menus are up, and you click on
the Activities button, then the menus go away, but it does not enter the
overview. This is essentially the same oddity as the (pre-existing) bug
that if you press on the menu title for the currently-open menu, that
menu hides immediately (rather than waiting for the button release). But
I'll fix that after bug 643687 lands.
Comment 8 Dan Winship 2011-07-14 15:05:32 UTC
*** Bug 641253 has been marked as a duplicate of this bug. ***
Comment 9 Dan Winship 2011-07-14 15:07:16 UTC
Created attachment 191969 [details] [review]
panel: abstract ActivitiesButton into its own class
Comment 10 Dan Winship 2011-07-14 15:07:28 UTC
Created attachment 191970 [details] [review]
panel: make ActivitiesButton a PanelMenu.Button

The fact that everything in the top bar except the activities button
was a menu made various things difficult. Simplify this by making the
activities button be a menu too, but just hack it up a bit so that the
menu associated with the button never actually appears.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=645759 (Clicking on
Activities with menu up leaves a funny state) and its semi-dup 641253
(panel keynav between Activities and menus is quirky).
Comment 11 Dan Winship 2011-07-14 15:07:42 UTC
Created attachment 191971 [details] [review]
panel, layout: clean up HotCorner handling

Move the HotCorner class from panel to layout, and make the panel
manage its own HotCorner.

Stick the panel's HotCorner into the Activities button actor (rather
than separately floating above it), so that hover tracking on the
button works properly without needing hacks in HotCorner.
Comment 12 Florian Müllner 2011-07-14 15:42:23 UTC
Review of attachment 191971 [details] [review]:

Not a full review, only pointing out the errors which prevent the code from running ...

::: js/ui/layout.js
@@ +194,3 @@
+        if (St.Widget.get_default_direction() == St.TextDirection.RTL) {
+            this._corner.set_position(this.actor.width - this._corner.width, 0);
+            this.actor.set_anchor_point_from_gravity(Clutter.Gravity.NORTH_EAST

Missing );

@@ +292,3 @@
+        if (this.shouldToggleOverviewOnClick())
+            Main.overview.toggle();
+        ret urn true;

...

::: js/ui/panel.js
@@ +568,3 @@
+        container.add_actor(this._label);
+
+        this._hotCorner = new HotCorner();

Layout.HotCorner?
Comment 13 Dan Winship 2011-07-14 15:46:09 UTC
I will not do a "trivial" rebase and then submit a patch without testing.
I will not do a "trivial" rebase and then submit a patch without testing.
I will not do a "trivial" rebase and then submit a patch without testing.
I will not do a "trivial" rebase and then submit a patch without testing.
...
Comment 14 Dan Winship 2011-07-14 15:56:42 UTC
Created attachment 191980 [details] [review]
panel, layout: clean up HotCorner handling

Move the HotCorner class from panel to layout, and make the panel
manage its own HotCorner.

Stick the panel's HotCorner into the Activities button actor (rather
than separately floating above it), so that hover tracking on the
button works properly without needing hacks in HotCorner.
Comment 15 Florian Müllner 2011-07-14 16:20:23 UTC
Review of attachment 191969 [details] [review]:

Looks good!

::: js/ui/panel.js
@@ +978,3 @@
         /* Button on the left side of the panel. */
+        this._activitiesButton = new ActivitiesButton();
+        this._activities = this.button = this._activitiesButton.actor;

Maybe a good time to get rid of the overly generic 'this.button'?
Comment 16 Florian Müllner 2011-07-14 17:29:58 UTC
Review of attachment 191970 [details] [review]:

Looking good

::: data/theme/gnome-shell.css
@@ +266,2 @@
 .panel-corner:active,
 .panel-corner:checked,

time to remove :checked?

@@ +304,2 @@
 .panel-button:active,
 .panel-button:checked,

Dto.
Comment 17 Florian Müllner 2011-07-14 17:55:16 UTC
Review of attachment 191980 [details] [review]:

Looks good except for a minor style breakage (when toggling the activities button, the highlight effect on the button is out-of-sync with the effect on corner).

::: js/ui/panel.js
@@ +565,3 @@
         /* Translators: If there is no suitable word for "Activities"
            in your language, you can use the word for "Overview". */
+        this._label = new St.Label({ name: 'panelActivities',

This change breaks the existing CSS - either the CSS needs adjustment, or the name should be left to this.actor.

@@ -1031,3 @@
         /* Button on the left side of the panel. */
         this._activitiesButton = new ActivitiesButton();
-        this._activities = this.button = this._activitiesButton.actor;

Ah, never mind the earlier comment about removing this.button.
Comment 18 Dan Winship 2011-07-14 19:31:57 UTC
pushed with the fixes you pointed out

Attachment 191969 [details] pushed as 544bdb4 - panel: abstract ActivitiesButton into its own class
Attachment 191970 [details] pushed as 50f248e - panel: make ActivitiesButton a PanelMenu.Button
Attachment 191980 [details] pushed as 0549f42 - panel, layout: clean up HotCorner handling