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 642005 - MessageTray: fix showing and hiding summary notifications when summary items are clicked
MessageTray: fix showing and hiding summary notifications when summary items ...
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other All
: Normal normal
: ---
Assigned To: Dan Winship
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-10 04:42 UTC by Marina Zhurakhinskaya
Modified: 2011-02-15 23:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MessageTray: fix showing and hiding summary notifications when summary items are clicked (4.36 KB, patch)
2011-02-10 04:42 UTC, Marina Zhurakhinskaya
reviewed Details | Review
MessageTray: fix showing and hiding summary notifications when summary items are clicked (3.81 KB, patch)
2011-02-15 23:40 UTC, Marina Zhurakhinskaya
committed Details | Review

Description Marina Zhurakhinskaya 2011-02-10 04:42:24 UTC
This patch depend on the patch from bug 641810.
Comment 1 Marina Zhurakhinskaya 2011-02-10 04:42:27 UTC
Created attachment 180553 [details] [review]
MessageTray: fix showing and hiding summary notifications when summary items are clicked

This patch fixes the summary notification reappearing if you click on the
summary item to hide it and hover away. It also ensures that when you click
on any summary item which doesn't correspond to the summary notification
being shown, a new summary notification will replace it right away.

What used to happen is that we'd unset the clicked item in _unlock() that
was called when the focus was ungrabbed because the user clicked outside
of the summary notification, but then would have this._clickedSummaryItem
be null in _onSummaryItemClicked() , and set it to the clicked item all
over again. This patch ensures that we unset the clicked item only when
it is necessary.

We also needed to add the code to call _updateState() again to show a new
summary notification when a previous one was hidden, but
this._clickedSummaryItem was set.
Comment 2 Dan Winship 2011-02-15 17:49:37 UTC
Comment on attachment 180553 [details] [review]
MessageTray: fix showing and hiding summary notifications when summary items are clicked

>+        this._focusGrabber.connect('button-pressed', Lang.bind(this,
>+           function(focusGrabber, source) {

This seems to be doing a lot more work than it needs. Given that ungrabFocus will no longer call _unsetClickedSummaryItem, it seems that all we need to do here is:

                 if (this._clickedSummaryItem && !this._clickedSummaryItem.actor.contains(source))
                     this._unsetClickedSummaryItem();
                 this._focusGrabber.ungrabFocus();
             }));

If the user clicks on the currently-selected summary item, we leave _clickedSummaryItem set (and so _onSummaryItemClicked will do the right thing). Otherwise we clear it. Either way, we ungrab.
Comment 3 Marina Zhurakhinskaya 2011-02-15 23:40:07 UTC
Created attachment 180954 [details] [review]
MessageTray: fix showing and hiding summary notifications when summary items are clicked

This patch fixes the summary notification reappearing if you click on the
summary item to hide it and hover away. It also ensures that when you click
on any summary item which doesn't correspond to the summary notification
being shown, a new summary notification will replace it right away.

What used to happen is that we'd unset the clicked item in _unlock() that
was called when the focus was ungrabbed because the user clicked outside
of the summary notification, but then would have this._clickedSummaryItem
be null in _onSummaryItemClicked() , and set it to the clicked item all
over again. This patch ensures that we unset the clicked item only when
it is necessary.

We also needed to add the code to call _updateState() again to show a new
summary notification when a previous one was hidden, but
this._clickedSummaryItem was set.