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 758182 - Use icons in the selection toolbar
Use icons in the selection toolbar
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-11-16 15:25 UTC by Alessandro Bono
Modified: 2015-11-30 16:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
selections: Remove useless GtkRevealer (4.01 KB, patch)
2015-11-16 15:26 UTC, Alessandro Bono
committed Details | Review
selections: Port to templates (7.25 KB, patch)
2015-11-16 15:27 UTC, Alessandro Bono
none Details | Review
selection-toolbar: Rearrange buttons and use icons (3.07 KB, patch)
2015-11-16 15:27 UTC, Alessandro Bono
none Details | Review
selections: Port to templates (7.21 KB, patch)
2015-11-17 14:31 UTC, Alessandro Bono
committed Details | Review
selection-toolbar: Rearrange buttons and use icons (3.03 KB, patch)
2015-11-17 14:31 UTC, Alessandro Bono
none Details | Review
selection-toolbar: Rearrange buttons and use icons (3.01 KB, patch)
2015-11-25 22:37 UTC, Alessandro Bono
committed Details | Review
selection-toolbar: Replace tabs with spaces (1.61 KB, patch)
2015-11-29 19:04 UTC, Alessandro Bono
committed Details | Review
selection-toolbar: Add missing packing info (844 bytes, patch)
2015-11-29 19:04 UTC, Alessandro Bono
committed Details | Review

Comment 1 Alessandro Bono 2015-11-16 15:26:59 UTC
Created attachment 315684 [details] [review]
selections: Remove useless GtkRevealer

It isn't necessary because GtkActionBar already contains a GtkRevealer.
Comment 2 Alessandro Bono 2015-11-16 15:27:17 UTC
Created attachment 315685 [details] [review]
selections: Port to templates
Comment 3 Alessandro Bono 2015-11-16 15:27:39 UTC
Created attachment 315686 [details] [review]
selection-toolbar: Rearrange buttons and use icons
Comment 4 Debarshi Ray 2015-11-16 15:39:56 UTC
Review of attachment 315684 [details] [review]:

Good catch.
Comment 5 Debarshi Ray 2015-11-16 15:57:41 UTC
Review of attachment 315685 [details] [review]:

Thanks, Alessandro. Looks good, apart from:

::: data/ui/selection-toolbar.ui
@@ +6,3 @@
+      <child>
+        <object class="GtkButton" id="toolbarOpen">
+          <property name="visible">True</property>

Please use 1 instead of 'True' and 0 instead of 'False'. I know it is not ideal, but otherwise gtk-builder-tool complains.

@@ +15,3 @@
+      <child>
+        <object class="GtkButton" id="toolbarPrint">
+          <property name="visible">True</property>

Ditto.

@@ +24,3 @@
+      <child>
+        <object class="GtkButton" id="toolbarTrash">
+          <property name="visible">True</property>

Ditto.
Comment 6 Matthias Clasen 2015-11-17 11:49:55 UTC
(In reply to Debarshi Ray from comment #5)
> Review of attachment 315685 [details] [review] [review]:
> 
> Thanks, Alessandro. Looks good, apart from:
> 
> ::: data/ui/selection-toolbar.ui
> @@ +6,3 @@
> +      <child>
> +        <object class="GtkButton" id="toolbarOpen">
> +          <property name="visible">True</property>
> 
> Please use 1 instead of 'True' and 0 instead of 'False'. I know it is not
> ideal, but otherwise gtk-builder-tool complains.
> 

It complains ? I don't think it complains. It may silently change a True to a 1, but it will not tell you that True is wrong.
Comment 7 Debarshi Ray 2015-11-17 13:30:27 UTC
(In reply to Matthias Clasen from comment #6)
> It complains ? I don't think it complains. It may silently change a True to
> a 1, but it will not tell you that True is wrong.

I meant that running 'gtk-builder-tool simplify' turns 'True' to 1.
Comment 8 Alessandro Bono 2015-11-17 14:31:09 UTC
Created attachment 315750 [details] [review]
selections: Port to templates
Comment 9 Alessandro Bono 2015-11-17 14:31:27 UTC
Created attachment 315751 [details] [review]
selection-toolbar: Rearrange buttons and use icons
Comment 10 Alessandro Bono 2015-11-17 14:40:58 UTC
Comment on attachment 315684 [details] [review]
selections: Remove useless GtkRevealer

Attachment 315684 [details] pushed as 13a001e - selections: Remove useless GtkRevealer
Comment 11 Alessandro Bono 2015-11-17 17:24:10 UTC
Comment on attachment 315750 [details] [review]
selections: Port to templates

Attachment 315750 [details] pushed as 21bfc56 - selections: Port to templates
Comment 12 Debarshi Ray 2015-11-25 12:16:46 UTC
Review of attachment 315751 [details] [review]:

Looks great. Feel free to push after making this minor tweak:

::: data/ui/selection-toolbar.ui
@@ +17,2 @@
           <property name="visible">1</property>
+          <property name="orientation">GTK_ORIENTATION_VERTICAL</property>

'vertical', like we do elsewhere
Comment 13 Alessandro Bono 2015-11-25 22:37:12 UTC
Created attachment 316275 [details] [review]
selection-toolbar: Rearrange buttons and use icons
Comment 14 Alessandro Bono 2015-11-25 22:38:32 UTC
Attachment 316275 [details] pushed as 1c0beeb - selection-toolbar: Rearrange buttons and use icons
Comment 15 Alessandro Bono 2015-11-29 19:04:02 UTC
Created attachment 316483 [details] [review]
selection-toolbar: Replace tabs with spaces
Comment 16 Alessandro Bono 2015-11-29 19:04:23 UTC
Created attachment 316484 [details] [review]
selection-toolbar: Add missing packing info
Comment 17 Debarshi Ray 2015-11-30 14:50:15 UTC
Review of attachment 316483 [details] [review]:

Of course.
Comment 18 Debarshi Ray 2015-11-30 14:52:15 UTC
Review of attachment 316484 [details] [review]:

++
Comment 19 Alessandro Bono 2015-11-30 16:38:57 UTC
Attachment 316483 [details] pushed as 47d6017 - selection-toolbar: Replace tabs with spaces
Attachment 316484 [details] pushed as 853bef0 - selection-toolbar: Add missing packing info