GNOME Bugzilla – Bug 679809
Close buttons on expanded notifications and bubbles
Last modified: 2012-08-20 23:01:00 UTC
Currently, it is hard to know how to close an expanded/bubble notification. It can also be inconvenient, since you have to move the pointer away or click on something else. It would be better if we displayed a close button for these types of notifications. This would provide a discoverable way to close the notification, and will help with some of the queuing plans that are described in bug 677217. Note that close buttons are needed on banner notifications. See: http://www.youtube.com/watch?v=14z4wdgNF9g
Created attachment 221723 [details] [review] Patch: add close button to notifications Here is a patch that Marina and I have come up with.
Review of attachment 221723 [details] [review]: ::: js/ui/messageTray.js @@ +1224,3 @@ + this.notificationStackWidget.width = this.notificationStackView.width; + this._closeButton.x = this.notificationStackView.width - overlap; + this._closeButton.y = - overlap; Using a ClutterBinLayout, you can set the child's alignment to be align_x=end, align_y=start, and then use the anchor_x/anchor_y properties to tweak the overlap. I think this might be a bit more clear. @@ +1225,3 @@ + this._closeButton.x = this.notificationStackView.width - overlap; + this._closeButton.y = - overlap; + this.scrollTo(St.Side.BOTTOM); Looks unrelated? @@ +1294,3 @@ + + if (notification.actor.get_parent() == this.notificationStack) + this.notificationStack.remove_actor(notification.actor); Looks unrelated as well? @@ +1365,3 @@ this._pointerBarrier = 0; + this._closeButton = new St.Button({ style_class: 'window-close' }); Hm, so we're creating one for the notification stack, and one for the message tray notification bin? Perhaps it's worth it to wrap the notificationBin in an StWidget as well, so that we can get the same layout cleanliness, and don't have to do all the dirty hacks as below. @@ +2156,3 @@ this._notificationBin.hide(); + this._closeButton.hide(); + this._pointerInTray = false; Unrelated? @@ +2189,3 @@ + // for this._notification.actor.x + this._notification.actor.get_allocation_box(); + this._closeButton.x = (this._notification.actor.x + this._notification.actor.width) - overlap; And then all of this can go away :) @@ +2213,3 @@ + }); + } else { + this._closeButton.y = expandedY - overlap; And all of this too. @@ -2191,3 @@ this._summaryBoxPointerItem.prepareNotificationStackForShowing(); - this._summaryBoxPointer.bin.child = this._summaryBoxPointerItem.notificationStackView; - this._summaryBoxPointerItem.scrollTo(St.Side.BOTTOM); Seems to be unrelated as well. @@ +2348,3 @@ _hideSummaryBoxPointer: function() { + // We should be sure to hide the box pointer if all notifications in it are destroyed while + // it is hiding, so that we don't show an animation of an empty blob being hidden. This is an unrelated, and potentially dangerous hunk.
Apologies, it's translation_x/translation_y. anchor_x/anchor_y are deprecated.
Comment on attachment 221723 [details] [review] Patch: add close button to notifications This patch was landed in suggested separate commits and the layout changes were implemented by Jasper in bug 682253.