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 778003 - The "Label File System" dialog is too small
The "Label File System" dialog is too small
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
0.27.0
Other Linux
: Normal enhancement
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2017-01-31 21:13 UTC by Refael Sheinker
Modified: 2017-02-17 23:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Its a patch that will fix the problem. (1.10 KB, application/mbox)
2017-01-31 21:13 UTC, Refael Sheinker
  Details
Its a patch that will fix the problem (1.56 KB, patch)
2017-02-01 14:29 UTC, Refael Sheinker
none Details | Review
Label File System screen shot using a long FAKE RAID device name (9.28 KB, image/png)
2017-02-01 17:28 UTC, Curtis Gedak
  Details
Bug demonstration (78.83 KB, image/png)
2017-02-01 18:46 UTC, Refael Sheinker
  Details
Its a patch that will fix the problem (1.64 KB, application/mbox)
2017-02-01 18:49 UTC, Refael Sheinker
  Details
Its a patch that will fix the problem (1.56 KB, patch)
2017-02-01 19:31 UTC, Refael Sheinker
none Details | Review
Its a patch that will fix the problem (1.56 KB, application/mbox)
2017-02-01 19:33 UTC, Refael Sheinker
  Details
Works on Debian Live CD (426.73 KB, image/jpeg)
2017-02-06 14:38 UTC, Refael Sheinker
  Details
Montage of Label dialogs, #1 (406.17 KB, patch)
2017-02-07 21:09 UTC, Mike Fleetwood
none Details | Review
Montage of Label dialogs, #1 (406.17 KB, image/png)
2017-02-08 20:26 UTC, Mike Fleetwood
  Details
Montage of Label dialogs, #2 (606.87 KB, image/png)
2017-02-12 09:56 UTC, Mike Fleetwood
  Details
Increase label dialog size (v4) (3.46 KB, patch)
2017-02-12 09:58 UTC, Mike Fleetwood
none Details | Review

Description Refael Sheinker 2017-01-31 21:13:35 UTC
Created attachment 344668 [details]
Its a patch that will fix the problem.

The "Label File System" dialog is too small.
Here is a screenshot:
https://www.dropbox.com/s/qt4a1aeih19i9qa/Screenshot_2017-01-31_22-59-37.png?dl=0

I am attaching a patch to fix the problem.

Thanks in advance :-)
Refael.
Comment 1 Mike Fleetwood 2017-02-01 13:57:13 UTC
Hi Refael,

Can you also make the label entry box 15 pixels wider too to match the
wider dialog, otherwise it looks a bit odd.
Also please update/set you full name in git so we have it for the author
of the patch (along with your email address which is already set).
Either globally:
    git config --global user.name "Refael Sheinker"
or per GIT repo:
    cd ${GPARTED_GIT_REPO}
    git config user.name "Refael Sheinker"

Thanks,
Mike
Comment 2 Refael Sheinker 2017-02-01 14:29:44 UTC
Created attachment 344714 [details] [review]
Its a patch that will fix the problem
Comment 3 Refael Sheinker 2017-02-01 14:32:05 UTC
(In reply to Mike Fleetwood from comment #1)
> Hi Refael,
> 
> Can you also make the label entry box 15 pixels wider too to match the
> wider dialog, otherwise it looks a bit odd.
> Also please update/set you full name in git so we have it for the author
> of the patch (along with your email address which is already set).
> Either globally:
>     git config --global user.name "Refael Sheinker"
> or per GIT repo:
>     cd ${GPARTED_GIT_REPO}
>     git config user.name "Refael Sheinker"
> 
> Thanks,
> Mike
Can you also make the label entry box 15 pixels - Excellent suggestion. Done :-)
Also please update/set you full name - Done :-)
I also uploaded the new patch and marked the old upload obsolete.
Comment 4 Curtis Gedak 2017-02-01 17:05:40 UTC
Hi Refael,

Thanks for your interest in improving GParted.

Following are two suggested changes to this bug report and patch.

1)  Would you be able to attach the screen shot to this bug report?

That way the screen shot would always be available in bugzilla, even if the dropbox image is deleted.

2)  Instead of linking to dropbox in the patch comment, it would be more informative to mention this bug report including the number and one-line description.

For instructions see the following gparted git development page:
http://gparted.org/git.php

For examples, you can run "git log" and notice the references to the bug reports.  These references become clickable links to the bug report when viewing the git history.

For a simple example see:

Add LUKS notes to the GParted Manual (#774818)
https://git.gnome.org/browse/gparted/commit/?id=5857b46c5d9918ae811398f2a151fa1512bfdd3a

Note the bottom two lines referencing the bug report.

Thanks,
Curtis
Comment 5 Curtis Gedak 2017-02-01 17:28:35 UTC
Created attachment 344724 [details]
Label File System screen shot using a long FAKE RAID device name

Attached is a screen shot using the patch from comment #2.  The screen shot was taken on my kubuntu 16.04 system while labeling a FAKE RAID device (/dev/mapper/isw_jedagafcd_Vol0).

Note that the full text of the window title does not fit.  If we wish to handle this case as well, then we might consider increasing the width of the dialog box further.

I realize that there may be situations where the length of the window title exceeds the width of display device.  With a web browser these longer titles are shortened with "..." as show in the attached screen shot.
Comment 6 Refael Sheinker 2017-02-01 18:46:56 UTC
Created attachment 344734 [details]
Bug demonstration
Comment 7 Refael Sheinker 2017-02-01 18:49:13 UTC
Created attachment 344735 [details]
Its a patch that will fix the problem
Comment 8 Refael Sheinker 2017-02-01 18:51:21 UTC
(In reply to Curtis Gedak from comment #4)
> Hi Refael,
> 
> Thanks for your interest in improving GParted.
> 
> Following are two suggested changes to this bug report and patch.
> 
> 1)  Would you be able to attach the screen shot to this bug report?
> 
> That way the screen shot would always be available in bugzilla, even if the
> dropbox image is deleted.
> 
> 2)  Instead of linking to dropbox in the patch comment, it would be more
> informative to mention this bug report including the number and one-line
> description.
> 
> For instructions see the following gparted git development page:
> http://gparted.org/git.php
> 
> For examples, you can run "git log" and notice the references to the bug
> reports.  These references become clickable links to the bug report when
> viewing the git history.
> 
> For a simple example see:
> 
> Add LUKS notes to the GParted Manual (#774818)
> https://git.gnome.org/browse/gparted/commit/
> ?id=5857b46c5d9918ae811398f2a151fa1512bfdd3a
> 
> Note the bottom two lines referencing the bug report.
> 
> Thanks,
> Curtis

1)  Would you be able to attach the screen shot to this bug report? - Done :-)

2)  Instead of linking to dropbox in the patch comment, it would be more
informative to mention this bug report including the number and one-line
description. - Done :-)
Comment 9 Refael Sheinker 2017-02-01 19:02:36 UTC
(In reply to Curtis Gedak from comment #5)
> Created attachment 344724 [details]
> Label File System screen shot using a long FAKE RAID device name
> 
> Attached is a screen shot using the patch from comment #2.  The screen shot
> was taken on my kubuntu 16.04 system while labeling a FAKE RAID device
> (/dev/mapper/isw_jedagafcd_Vol0).
> 
> Note that the full text of the window title does not fit.  If we wish to
> handle this case as well, then we might consider increasing the width of the
> dialog box further.
> 
> I realize that there may be situations where the length of the window title
> exceeds the width of display device.  With a web browser these longer titles
> are shortened with "..." as show in the attached screen shot.

Good point. Two observations:
1. "I realize that there may be situations where the length of the window title exceeds the width of display device" - I find it hard to imagine a PC with such a small screen. What did You meant?
2. Maybe We should put the text not on the window decoration but on the dialog itself?
Comment 10 Curtis Gedak 2017-02-01 19:15:46 UTC
Hi Refael,

(In reply to Refael Sheinker from comment #9)
> 1. "I realize that there may be situations where the length of the window
> title exceeds the width of display device" - I find it hard to imagine a PC
> with such a small screen. What did You meant?

For GParted we use 800x600 as our target minimum screen resolution.  When the value of a window title is a variable, it is possible that depending on the font used and variable contents that these might extend beyond the size of the window and potentially the entire monitor screen.

This is a hypothetical situation.  To be practical we often design solutions for the most common use case.  I only raised this issue to point out a potential issue.  I do not think we need to address it.

> 2. Maybe We should put the text not on the window decoration but on the
> dialog itself?

That is certainly one way to work around the issue.

For this patch, I think the sizes chosen are okay.  No need to restructure the dialog box.


On the topic of the git comment, I was looking for something without the dropbox link that looks more like the following:


Make "Label File System" dialog a bit bigger (#778003)

The "Label File System" dialog looks too small.
This problem is that the label field clips the Cancel and OK buttons.

Bug 778003 - The "Label File System" dialog is too small


Regards,
Curtis
Comment 11 Refael Sheinker 2017-02-01 19:31:12 UTC
Created attachment 344737 [details] [review]
Its a patch that will fix the problem
Comment 12 Refael Sheinker 2017-02-01 19:33:12 UTC
Created attachment 344738 [details]
Its a patch that will fix the problem
Comment 13 Refael Sheinker 2017-02-01 19:33:39 UTC
(In reply to Curtis Gedak from comment #10)
> For this patch, I think the sizes chosen are okay.  No need to restructure
> the dialog box.

Agreed.

> On the topic of the git comment, I was looking for something without the
> dropbox link that looks more like the following:
> 
> 
> Make "Label File System" dialog a bit bigger (#778003)
> 
> The "Label File System" dialog looks too small.
> This problem is that the label field clips the Cancel and OK buttons.
> 
> Bug 778003 - The "Label File System" dialog is too small

I uploaded a patch with the changes You suggested. Thanks :-)
Comment 14 Mike Fleetwood 2017-02-02 15:23:38 UTC
Hi Refael,

What distro/desktop/version are you using?
I tried a few but didn't get the entry box overlapping with the buttons?

Thanks,
Mike
Comment 15 Refael Sheinker 2017-02-02 15:27:06 UTC
(In reply to Mike Fleetwood from comment #14)
> What distro/desktop/version are you using?
Distro: Arch Linux; Desktop: XFCE v4.12; GParted: v0.27.0;

> I tried a few but didn't get the entry box overlapping with the buttons?
I had the feeling that I am the only one seeing it :-)
Comment 16 Refael Sheinker 2017-02-06 14:38:44 UTC
Created attachment 345032 [details]
Works on Debian Live CD
Comment 17 Refael Sheinker 2017-02-06 14:41:11 UTC
I've just uploaded another screenshot.
Its from Debian Live CD 8.7.0.
Gparted is v0.19.0-2.
As can be seen in the screenshot, the text box does not overlap the buttons,
e.g., the bug reproduces only on My PC (it seems so any way).
Comment 18 Refael Sheinker 2017-02-06 14:43:24 UTC
(In reply to Refael Sheinker from comment #17)
> I've just uploaded another screenshot.
Forgot to mention that the attachment id is 345032,
here is the direct link for convenience:
https://bug778003.bugzilla-attachments.gnome.org/attachment.cgi?id=345032
Comment 19 Mike Fleetwood 2017-02-06 14:57:48 UTC
Hi Refael,

Sorry for not replying sooner.  I have still not fully checked the
layout of the label dialog before and after on a range of distros yet.

I have also found that the label entry box overlaps with the buttons on
Fedora 24 and Fedora Rawhide.  It is not just on your Arch Linux box.  I
suspect it is a font/spacing layout change in GTK/GNOME which is causing
it.

Initial indication is that we probably need to stop specifying the
height of the label dialog and just let it be derived automatically.
Update to follow.

Mike
Comment 20 Refael Sheinker 2017-02-06 15:10:28 UTC
(In reply to Mike Fleetwood from comment #19)
> Hi Refael,
> 
> Sorry for not replying sooner. 
No pressure :-) Take Your time :-)
> I have still not fully checked the
> layout of the label dialog before and after on a range of distros yet.
> 
> I have also found that the label entry box overlaps with the buttons on
> Fedora 24 and Fedora Rawhide.  It is not just on your Arch Linux box.  I
> suspect it is a font/spacing layout change in GTK/GNOME which is causing
> it.
> 
> Initial indication is that we probably need to stop specifying the
> height of the label dialog and just let it be derived automatically.
> Update to follow.

Fascinating!
I will eagerly await for any update :-)
Tank You :-)
Comment 21 Mike Fleetwood 2017-02-07 21:09:36 UTC
Created attachment 345146 [details] [review]
Montage of Label dialogs, #1

Hi All,

So here is a montage of the Label dialog showing before and after on a
range of distros.

The montage contains:
  Top row:    [before] GParted 0.27.0
  Bottom row: [after]  Refael's patch from comment #11
  From left to right:
    CentOS 6      CentOS 7      Ubuntu 16.04 LTS    Fedora 24
    GNOME 2.30    GNOME 3.14    XFCE 4.12           GNOME 3.20

As can be seen different distros/desktops use different fonts which
makes a big difference on the size of the "Label:" and entry box widgets
within the dialog.

1) Definitely need to not set the height of the dialog, instead letting
   it fit the combined height of all the widgets automatically.
   (Pass height=-1 to set_size_request()).

2) The dialog width and entry box width: definitely need to change them
   because on CentOS 6 the entry box disappears of the edge of the
   dialog.  Just not sure to what yet.

Mike
Comment 22 Refael Sheinker 2017-02-08 12:06:06 UTC
(In reply to Mike Fleetwood from comment #21)
> Created attachment 345146 [details] [review] [review]
> Montage of Label dialogs, #1

I am terribly sorry, but the attachment does not seems to be an image, Chrome opens it as a binary file.

> 
> Hi All,
> 
> So here is a montage of the Label dialog showing before and after on a
> range of distros.
> 
> The montage contains:
>   Top row:    [before] GParted 0.27.0
>   Bottom row: [after]  Refael's patch from comment #11
>   From left to right:
>     CentOS 6      CentOS 7      Ubuntu 16.04 LTS    Fedora 24
>     GNOME 2.30    GNOME 3.14    XFCE 4.12           GNOME 3.20
> 
> As can be seen different distros/desktops use different fonts which
> makes a big difference on the size of the "Label:" and entry box widgets
> within the dialog.
> 
> 1) Definitely need to not set the height of the dialog, instead letting
>    it fit the combined height of all the widgets automatically.
>    (Pass height=-1 to set_size_request()).

Cool! Excellent solution.

> 2) The dialog width and entry box width: definitely need to change them
>    because on CentOS 6 the entry box disappears of the edge of the
>    dialog.  Just not sure to what yet.

Why not just make it some arbitrary big (-ish) value and be done with it?
For example, 400, nice and round number.

> Mike
Comment 23 Curtis Gedak 2017-02-08 17:00:02 UTC
Hi Mike,

Thank you for delving deeper into this issue.

I too was unable to view the image.  It appears that the .png image in comment #12 was incorrectly interpreted as "text/plain" in the attachment.

Perhaps you could upload it again and override the type, setting it to "PNG image"?

Curtis
Comment 24 Mike Fleetwood 2017-02-08 20:26:05 UTC
Created attachment 345267 [details]
Montage of Label dialogs, #1

This time marking the attachment as an image file instead of a patch!
Comment 25 Mike Fleetwood 2017-02-12 09:56:07 UTC
Created attachment 345559 [details]
Montage of Label dialogs, #2

Created this montage of different choices for the width of the label
file system dialog to see what "looked right".

Montage #2 contains:
  Top row:    height = auto(-1)  [1]
  Middle row: height = auto(-1), box = 30 chars, width = auto(-1)
  Bottom row: height = auto(-1), box = 30 chars, width = 400 pxls
  From left to right:
    CentOS 6      CentOS 7      Ubuntu 16.04 LTS    Fedora 24
    GNOME 2.30    GNOME 3.14    XFCE 4.12           GNOME 3.20

  [1] width values as found in GParted 0.27.0, box = 20 chars,
      width = 300 pxls

Mike
Comment 26 Mike Fleetwood 2017-02-12 09:58:09 UTC
Created attachment 345560 [details] [review]
Increase label dialog size (v4)

Hi All,


I decided to go with Refael's suggestion of 400 pxl wide dialog and for
a 30 char wide entry box (the bottom row).

I have looked at a few different choices of setting the dialog width and
label entry box width and decided with Refael's suggestion; dialog width
400 pxls and label entry box width 30 chars.

As soon as GParted code change embargo is over after the release on
Tuesday 14th February I will be pushing this patchset unless I hear
otherwise.

Thank you Refael for you patch and patience in this regard,
Mike
Comment 27 Curtis Gedak 2017-02-13 18:01:17 UTC
Hi All,

One question on the actual patch.  It might just be me, but the patch in comment #26 seems to set the same two lines twice.  One in 1/2 and again in 2/2.  The only difference appears to be the text in the commit comment.

More specifically,

    this->set_size_request( 400, -1 );

and 

    entry->set_width_chars( 30 );

are the two lines in question.


Am I missing something?

Should this patch simply be merged into one commit?

Thanks,
Curtis
Comment 28 Mike Fleetwood 2017-02-13 18:09:39 UTC
(In reply to Curtis Gedak from comment #27)
> One question on the actual patch.  It might just be me, but the patch in
> comment #26 seems to set the same two lines twice.  One in 1/2 and again in
> 2/2.  The only difference appears to be the text in the commit comment.
> 
> More specifically,
> 
>     this->set_size_request( 400, -1 );
> 
> and 
> 
>     entry->set_width_chars( 30 );
> 
> are the two lines in question.

this->set_size_request() is setting the size of the dialog in pixels.
entry->set_width_chars() is setting the size of the entry box widget
within the dialog to be wide enough for about that many characters.

Mike
Comment 29 Curtis Gedak 2017-02-13 18:15:23 UTC
Thank you Mike for the prompt response.  It seems I was not precise in stating my question.  I will try again and I appreciate your patience.


The first 1/2 patch v4 sets both:

    this->set_size_request( 400, -1 );
and
    entry->set_width_chars( 30 );


Then the next 2/2 patch v4 again sets both

    this->set_size_request( 400, -1 );
and
    entry->set_width_chars( 30 );



It looks to me like these two lines are being set in the first patch, and then set again in the second patch.

My question is why are there two patches that seem to do the same thing?

Thanks,
Curtis
Comment 30 Mike Fleetwood 2017-02-13 18:23:07 UTC
The first patch changes the Label File System dialog and the second
patch changes the Name Partition dialog.  Very similar code to do a very
similar job in two separate classes and dialogs.
Comment 31 Curtis Gedak 2017-02-13 18:27:10 UTC
Thank you Mike for this clarification.  I didn't see that these two were different dialog windows (me face-palm).  That was my challenge.  :-)

The patch set looks good to me and can be applied after we create our 0.28.0 official release on Feb 14th.

Curtis
Comment 32 Mike Fleetwood 2017-02-14 19:01:04 UTC
Hi Refael,

The patchset from comment #26 above has been pushed to the upstream GIT
repository ready for the next release of GParted.

Make "Label File System" dialog a bit bigger (#778003)
https://git.gnome.org/browse/gparted/commit/?id=d1ce653d1b71c987a2801537ca60be5ff2465699

Make the Name Partition dialog a bit bigger (#778003)
https://git.gnome.org/browse/gparted/commit/?id=925e505f77802955f831b8d32b390ad24ee3db07

Thanks,
Mike
Comment 33 Refael Sheinker 2017-02-14 19:17:33 UTC
(In reply to Mike Fleetwood from comment #32)
> Hi Refael,
> 
> The patchset from comment #26 above has been pushed to the upstream GIT
> repository ready for the next release of GParted.
> 
> Make "Label File System" dialog a bit bigger (#778003)
> https://git.gnome.org/browse/gparted/commit/
> ?id=d1ce653d1b71c987a2801537ca60be5ff2465699
> 
> Make the Name Partition dialog a bit bigger (#778003)
> https://git.gnome.org/browse/gparted/commit/
> ?id=925e505f77802955f831b8d32b390ad24ee3db07
> 
> Thanks,
> Mike

Cool :-)
Thank You every one! And thank God for open source :-)
Comment 34 Curtis Gedak 2017-02-17 23:10:58 UTC
This enhancement was included in the GParted 0.28.1 release on February 17, 2017.