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 771813 - Use a revelaer in batch rename dialog for the automatic numbering
Use a revelaer in batch rename dialog for the automatic numbering
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-22 08:26 UTC by Carlos Soriano
Modified: 2016-10-03 09:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
batch-rename-dialog: revealer for auto numbering (7.08 KB, patch)
2016-09-27 23:03 UTC, Sirbu Lavinia Stefania
none Details | Review
batch-rename-dialog: revealer for auto numbering (8.57 KB, patch)
2016-09-29 18:20 UTC, Sirbu Lavinia Stefania
none Details | Review
batch-rename-dialog: revealer for auto numbering (8.54 KB, patch)
2016-09-29 23:15 UTC, Sirbu Lavinia Stefania
committed Details | Review

Description Carlos Soriano 2016-09-22 08:26:02 UTC
Currently we have a big padding there, with no pourpose. Would be good to have a revealer and only use that space if needed.
Comment 1 Carlos Soriano 2016-09-22 08:26:55 UTC
Intended for newcomers
Comment 2 Sirbu Lavinia Stefania 2016-09-27 23:03:21 UTC
Created attachment 336393 [details] [review]
batch-rename-dialog: revealer for auto numbering

Currently we have a big padding that initially has no meaning.

The problem is that when using the rename for multiple files, we initially
have an extra padding due to auto numbering. Even if the automatic numbering
is not used, the space for the toggle button used for the numbering options
is now blank and waiting for a possible use of the automatic option.

Add a GtkReaveler that contains the toggle button and reveals his child only
when necesarry, when the automatic numbering option is added.
Comment 3 Carlos Soriano 2016-09-29 07:53:06 UTC
Review of attachment 336393 [details] [review]:

Hey, thanks for the patch!

I noticed a few problems, did you see them?
When revealing the revealer it doesn't animate, however it does when unrevealing it.
Also the size of the numbering combobox changes just before animating to unreveal it, which is erroneous for achiving a smooth animation.

You can use the GtkInspector [0] to put a longer transition-duration in the revealer to notice more clearly these issues.

Worth to note that, size changes are usually because you are hiding/showing a widget, which may be not necessary if you are using a revealer, since the revealer already handles all the visibility of the widgets inside it when reveal/unreveal.

You can play a little more with revealers and see some examples using the gtk3-demo program, or in the gtk+ repo [1] for better understanding.

In case something is not clear (understanding the gtkrevealer can be a little difficult), don't hesitate to ask me/us on #nautilus IRC.

[0] https://wiki.gnome.org/Projects/GTK%2B/Inspector
[1] https://git.gnome.org/browse/gtk+/tree/tests/testrevealer.c run it building gtk+ and then "jhbuild run ./tests/.libs/testrevealer"
Comment 4 Sirbu Lavinia Stefania 2016-09-29 18:20:25 UTC
Created attachment 336535 [details] [review]
batch-rename-dialog: revealer for auto numbering

Currently we have a big padding that initially has no meaning.

The problem is that when using the rename for multiple files, we initially
have an extra padding due to auto numbering. Even if the automatic numbering
is not used, the space for the toggle button used for the numbering options
is now blank and waiting for a possible use of the automatic option.

Add a GtkReaveler that contains the toggle button and reveals his child only
when necesarry, when the automatic numbering option is added.
Comment 5 Carlos Soriano 2016-09-29 20:34:13 UTC
The issue was not the same dnd code path, but similar. So we coulf fix this one through reworking the clipboard handling of Nautilus

Attachment 336535 [details] pushed as b379507 - batch-rename-dialog: revealer for auto numbering
Comment 6 Carlos Soriano 2016-09-29 20:35:23 UTC
(In reply to Carlos Soriano from comment #5)
> The issue was not the same dnd code path, but similar. So we coulf fix this
> one through reworking the clipboard handling of Nautilus
> 
> Attachment 336535 [details] pushed as b379507 - batch-rename-dialog:
> revealer for auto numbering

whops, pushed and commented in the wrong bug :/
Comment 7 Carlos Soriano 2016-09-29 20:35:49 UTC
Comment on attachment 336535 [details] [review]
batch-rename-dialog: revealer for auto numbering

reopening
Comment 8 Carlos Soriano 2016-09-29 21:34:12 UTC
Review of attachment 336535 [details] [review]:

Hey, you got right how the revealer works, nice!

I saw your question in IRC, indeed this use of GtkGrid and packing is not right :( I hope it didn't confuse you much.
We can change that in a different patch.

Just some small questions and nitpicks:

::: src/resources/ui/nautilus-batch-rename-dialog.ui
@@ +169,2 @@
                                 <property name="can_focus">False</property>
+                                <property name="height-request">35</property>

why setting a size request? In most of the cases, this should be avoided, and let the widgets resize to the size they request for.

@@ +169,3 @@
                                 <property name="can_focus">False</property>
+                                <property name="height-request">35</property>
+                                <property name="sensitive">False</property>

why this change? A sensitive or insensitive label doesn't usually make much sense.

@@ +184,3 @@
+                                      <object class="GtkLabel" id="numbering_order_label">
+                                        <property name="visible">True</property>
+                                    <property name="orientation">horizontal</property>

In case you were looking at this as an example, this is wrong :( I will change it in another patch.
Comment 9 Sirbu Lavinia Stefania 2016-09-29 23:15:43 UTC
Created attachment 336573 [details] [review]
batch-rename-dialog: revealer for auto numbering

Currently we have a big padding that initially has no meaning.

The problem is that when using the rename for multiple files, we initially
have an extra padding due to auto numbering. Even if the automatic numbering
is not used, the space for the toggle button used for the numbering options
is now blank and waiting for a possible use of the automatic option.

Add a GtkReaveler that contains a child represented by a GtkBox. The GtkBox
contains a label and the toggle button for choosing an option of automatic
renaming. The child is revealed only when necesarry, more specifically when
the automatic numbering option is added.
Comment 10 Carlos Soriano 2016-10-03 09:27:59 UTC
Review of attachment 336573 [details] [review]:

Looks good now, thanks for your patch!
Comment 11 Carlos Soriano 2016-10-03 09:29:50 UTC
Attachment 336573 [details] pushed as 568193e - batch-rename-dialog: revealer for auto numbering