GNOME Bugzilla – Bug 756325
Roomlist: Highlight the room row I have right clicked
Last modified: 2015-10-28 20:01:28 UTC
Created attachment 313000 [details] picture of current behavior and suggested behavior When hovering over a row, there is a nice grey highlight on it. However, when I right-click the row the highlight dissapears as the right-click popup appears. We should keep the highlight around until I close the right-click popup again, so it is visually clear what room the right-click popup is connected to.
I'm working on this bug.
Created attachment 313905 [details] [review] Bug 756325 - Roomlist: Highlight the room row I have right clicked
Review of attachment 313905 [details] [review]: Apart from the issues pointed out below, please provide a more detailed commit message - https://wiki.gnome.org/Newcomers/CodeContributionWorkflow#Commit_guidelines has some guidelines. This is not a very complicated change, so it doesn't have to be very long, but please - outline the reason why the change is made - try to be precise (it's about showing the popover, not right-click - <shift>F10 also brings up the popover) ::: data/resources/application.css @@ +73,3 @@ } +.polari-room-list .list-row.has-open-popover{ Style nit: space before brace @@ +74,3 @@ +.polari-room-list .list-row.has-open-popover{ + background-color: @polari_dark_bg_color; This works fine for "normal" rows, but we don't want it for the selected one, see GtkPlacesSidebar for reference (run "jhbuild run gtk3-demo", run the "picker" demo and click the "file" button) ::: src/roomList.js @@ +26,3 @@ this._popover = Gtk.Popover.new_from_model(this.widget, menu); + this._popover.connect('notify::visible', Lang.bind(this, + function() { Indentation is four spaces, not tab. @@ +29,3 @@ + let roomContext = this.widget.get_style_context(); + if (this._popover.visible) + roomContext.add_class('has-open-popover'); 'has-open-popover' makes more sense than 'has-open-popup', but I would still use the latter, because it's used by GtkPlacesSidebar - if we can get the designers to support it for any GtkListRow, we get the CSS for free.
Created attachment 314340 [details] [review] Roomlist: Highlight the room row which the popup connects to Should be visually clear what room the opened popup is connected to. This fix highlights the room which the popup belongs to.
Review of attachment 314340 [details] [review]: Looks good to me - I'll push with the changes pointed out below ::: data/resources/application.css @@ +75,3 @@ +.polari-room-list .list-row.has-open-popup { + background-color: @polari_dark_bg_color; +} We only need those changes on the 3-18 branch - for master, Adwaita already provides all the styling for the .has-open-popup class we need. ::: src/roomList.js @@ +30,3 @@ + if (this._popover.visible) + roomContext.add_class('has-open-popup'); + else Nit: trailing whitespace
Attachment 314340 [details] pushed as f5dfe9b - Roomlist: Highlight the room row which the popup connects to