GNOME Bugzilla – Bug 778003
The "Label File System" dialog is too small
Last modified: 2017-02-17 23:10:58 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.
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
Created attachment 344714 [details] [review] Its a patch that will fix the problem
(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.
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
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.
Created attachment 344734 [details] Bug demonstration
Created attachment 344735 [details] Its a patch that will fix the problem
(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 :-)
(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?
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
Created attachment 344737 [details] [review] Its a patch that will fix the problem
Created attachment 344738 [details] Its a patch that will fix the problem
(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 :-)
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
(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 :-)
Created attachment 345032 [details] Works on Debian Live CD
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).
(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
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
(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 :-)
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
(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
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
Created attachment 345267 [details] Montage of Label dialogs, #1 This time marking the attachment as an image file instead of a patch!
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
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
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
(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
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
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.
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
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
(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 :-)
This enhancement was included in the GParted 0.28.1 release on February 17, 2017.