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 727257 - Bookmarks/contents dialog - close button
Bookmarks/contents dialog - close button
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
3.12.x
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
ready
Depends on:
Blocks:
 
 
Reported: 2014-03-28 17:04 UTC by Allan Day
Modified: 2014-04-03 11:12 UTC
See Also:
GNOME target: ---
GNOME version: 3.11/3.12


Attachments
Replaces Close button with standard x from bookmarks/contents dialog (1009 bytes, patch)
2014-04-02 13:44 UTC, Marta Milakovic
reviewed Details | Review
Replace "Close" button with standard "x" from bookmarks/contents dialog (1.62 KB, patch)
2014-04-02 17:02 UTC, Marta Milakovic
committed Details | Review

Description Allan Day 2014-03-28 17:04:44 UTC
The bookmarks/contents dialog has a "Close" button in the top right of the header bar. As this is a presentation dialog, the close button shouldn't have a text label - it should have a close button like standard window header bars.

See:

https://wiki.gnome.org/Design/HIG/Dialogs#Presentation_Dialogs
Comment 1 Marta Milakovic 2014-04-02 13:44:02 UTC
Created attachment 273478 [details] [review]
Replaces Close button with standard x from bookmarks/contents dialog
Comment 2 Debarshi Ray 2014-04-02 15:39:50 UTC
Review of attachment 273478 [details] [review]:

Thanks for the nice patch Marta. Could you please add the URL of this bug to the commit message? (You could try out git-bz.)

::: src/places.js
@@ -54,3 @@
                                         title: "",
                                         hexpand: true });
-        this.widget.add_button(_("Close"), Gtk.ResponseType.CLOSE);

Maybe we should use DELETE_EVENT instead of CLOSE a few lines below in _handleLink and _handleBookmark for the sake of consistency?
Comment 3 Marta Milakovic 2014-04-02 17:02:15 UTC
Created attachment 273487 [details] [review]
Replace "Close" button with standard "x" from bookmarks/contents dialog

Updated!
Bookmarks/contents dialog is a presentation dialog and it shouldn't have
close button with the label "Close". This patch adds standard close_button
to the dialog.
Comment 4 Marta Milakovic 2014-04-02 17:05:30 UTC
Thank you Debarshi for the review and advice. From now on only git-bz... it's great :)
Comment 5 Debarshi Ray 2014-04-03 11:10:31 UTC
Review of attachment 273487 [details] [review]:

Perfect. Thanks, Marta. I tweaked your commit message a bit because we don't need the "Updated!" in there.
Comment 6 Marta Milakovic 2014-04-03 11:12:36 UTC
Great, thank you!