Bug Hunting Session
Bug 105371 - Find & Replace Dialog: tune up
Summary: Find & Replace Dialog: tune up
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
5.3.0.1 rc
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Heiko Tietze
URL:
Whiteboard: target:5.4.0
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-16 17:49 UTC by poma
Modified: 2017-02-08 18:37 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Label "Replace:" (36 bytes, text/plain)
2017-01-16 17:49 UTC, poma
Details
Label "Replace:" (300.57 KB, image/png)
2017-01-16 17:53 UTC, poma
Details
Labels "Find & Replace" need more width (104.76 KB, image/png)
2017-01-17 08:15 UTC, poma
Details
Find & Replace Dialog: Make room for the longest translated labels (1.61 KB, patch)
2017-01-17 16:38 UTC, poma
Details
Find & Replace labels v2 (106.03 KB, image/png)
2017-01-17 16:40 UTC, poma
Details
Scaling 2x (200.32 KB, image/png)
2017-01-17 18:24 UTC, poma
Details
F&R on Windows (13.99 KB, image/png)
2017-01-18 08:18 UTC, Heiko Tietze
Details
Find & Replace: Make room for the longest translated labels and Similarities button (2.21 KB, patch)
2017-01-18 20:09 UTC, poma
Details
Similarities button destiny (72.66 KB, image/png)
2017-01-18 20:12 UTC, poma
Details
Find & Replace: Make room for the longest translated labels, Similarities and Sounds buttons (2.77 KB, patch)
2017-01-19 21:57 UTC, poma
Details
Wine 2.0-rc4 - Find & Replace - LibreOffice 5.3.0.2 (143.81 KB, image/png)
2017-01-19 21:58 UTC, poma
Details
Reworked dialog on gtk3 (46.67 KB, image/png)
2017-01-21 12:37 UTC, Heiko Tietze
Details
LO 5.3.0.3 - Boutons Rechercher (177.26 KB, image/png)
2017-01-29 19:07 UTC, poma
Details

Note You need to log in before you can comment on or make changes to this bug.
Description poma 2017-01-16 17:49:08 UTC
Created attachment 130477 [details]
Label "Replace:"
Comment 1 poma 2017-01-16 17:53:09 UTC
Created attachment 130478 [details]
Label "Replace:"

Label "Replace:" needs more width.
Comment 2 Heiko Tietze 2017-01-17 07:51:18 UTC
Right, an issue with sized font. But no explicit UX needed here. Sounds like the alignment is not well configured.

Could you please add a few words on your system, meaning desktop environment,  theme, font size, special settings etc. Because it looks not bad here.
Comment 3 poma 2017-01-17 08:15:46 UTC
Created attachment 130488 [details]
Labels "Find & Replace" need more width

Both, label "Find:" and label "Replace:", need more width - translation wise.
Comment 4 poma 2017-01-17 09:00:39 UTC
(In reply to Heiko Tietze from comment #2)
> Right, an issue with sized font. But no explicit UX needed here. Sounds like
> the alignment is not well configured.
> 
> Could you please add a few words on your system, meaning desktop
> environment,  theme, font size, special settings etc. Because it looks not
> bad here.

As can be seen in the first PNG attachment,
Desktop environments are Xfce, GNOME and KDE,
within Fedora - a Linux based operating system.

The relevant settings are the same:
- Font size: 12
- PPI (aka DPI): 120

Desktop themes aka styles:
- Duskgrey @ Xfce
- Adwaita @ GNOME 
- Breeze @ KDE

SAL_USE_VCLPLUGIN - VCL UI interfaces engaged:
- gtk @ Xfce
- gtk3 @ GNOME
- kde4 @ KDE
Comment 5 poma 2017-01-17 16:38:51 UTC
Created attachment 130499 [details]
Find & Replace Dialog: Make room for the longest translated labels
Comment 6 poma 2017-01-17 16:40:08 UTC
Created attachment 130500 [details]
Find & Replace labels v2
Comment 7 Heiko Tietze 2017-01-17 16:44:27 UTC
(In reply to poma from comment #5)
> Created attachment 130499 [details]
> Find & Replace Dialog: Make room for the longest translated labels

The better approach than <property name="width_request">142</property> would have been to set an alignment depending on the font size. If you work on a highres display or scale your font the 142px may be not enough or too large, respectively.
Comment 8 poma 2017-01-17 18:24:04 UTC
(In reply to Heiko Tietze from comment #7)
> (In reply to poma from comment #5)
> > Created attachment 130499 [details]
> > Find & Replace Dialog: Make room for the longest translated labels
> 
> The better approach than <property name="width_request">142</property> would
> have been to set an alignment depending on the font size. If you work on a
> highres display or scale your font the 142px may be not enough or too large,
> respectively.

I guess, there is always a more optimal solution.
Comment 9 poma 2017-01-17 18:24:52 UTC
Created attachment 130502 [details]
Scaling 2x
Comment 10 Heiko Tietze 2017-01-18 08:18:13 UTC
Created attachment 130517 [details]
F&R on Windows

Hijacking this thread as you also keep posting :-). Please have a look on the button "Similarities..." button. Sizing the buttons according the checkbox wasn't well-conceived.

The distance between labels and input fields is well done, btw.

Version: 5.4.0.0.alpha0+
Build ID: fa2eb4b43fc872c171129d477cfabe9fa29d78ce
CPU Threads: 4; OS Version: Windows 6.1; UI Render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2017-01-18_01:17:32
Locale: de-DE (de_DE); Calc: group
Comment 11 poma 2017-01-18 20:09:57 UTC
Created attachment 130538 [details]
Find & Replace: Make room for the longest translated labels and Similarities button
Comment 12 poma 2017-01-18 20:12:26 UTC
Created attachment 130539 [details]
Similarities button destiny
Comment 13 poma 2017-01-18 20:22:03 UTC
$ libreoffice --version
LibreOffice 5.3.0.2 30(Build:2)
Comment 14 Heiko Tietze 2017-01-19 09:13:03 UTC
(In reply to poma from comment #12)
> Created attachment 130539 [details]
> Similarities button destiny

Looks good in the lower picture.
Comment 15 poma 2017-01-19 16:26:01 UTC
If there is value, be free to apply a patch to the tree, you or someone else who can.

There are also other fine adjustments which can be applied within various Glade files,
in 'svx/uiconfig/ui/', however more on that in another bug report.
Comment 16 poma 2017-01-19 21:57:30 UTC
Created attachment 130562 [details]
Find & Replace: Make room for the longest translated labels, Similarities and Sounds buttons
Comment 17 poma 2017-01-19 21:58:38 UTC
Created attachment 130563 [details]
Wine 2.0-rc4 - Find & Replace - LibreOffice 5.3.0.2
Comment 18 poma 2017-01-19 22:24:19 UTC
As a reference,
after Wine (https://winehq.org) installs 'LibreOffice_5.3.0.2_Win_x86.msi',
"Find & Replace" UI config file is parked here:
~/.wine/drive_c/Program\ Files/LibreOffice\ 5/share/config/soffice.cfg/svx/ui/findreplacedialog.ui
linked as:
~/.wine/dosdevices/c\:/Program\ Files/LibreOffice\ 5/share/config/soffice.cfg/svx/ui/findreplacedialog.ui

Heiko, you can test patch yourself, without compiling an entire libreoffice, just (re)start it after changing 'findreplacedialog.ui'.
Comment 19 Heiko Tietze 2017-01-20 09:34:18 UTC
(In reply to poma from comment #18)
> Heiko, you can test patch yourself, without compiling an entire libreoffice,
> just (re)start it after changing 'findreplacedialog.ui'.

Please submit the patch to gerrit to allow cherrypicking. 
The screenshot looks good. But I'm afraid you place controls at a fix position, and that won't work for all label sizes, screen resolutions, and font scaling.
Comment 20 poma 2017-01-20 11:15:06 UTC
(In reply to Heiko Tietze from comment #19)
> (In reply to poma from comment #18)
> > Heiko, you can test patch yourself, without compiling an entire libreoffice,
> > just (re)start it after changing 'findreplacedialog.ui'.
> 
> Please submit the patch to gerrit to allow cherrypicking. 

http://devcentral.libreoffice.org
<quote>
Gerrit is the tool LibreOffice developers use for code review. This is where developers submit and review patches.
</quote>

This looks like a tool exclusively for developers, not for users.

> The screenshot looks good. But I'm afraid you place controls at a fix
> position, and that won't work for all label sizes, screen resolutions, and
> font scaling.

Can you share your patch here, because in itself a comment is not clear enough.
Comment 21 Heiko Tietze 2017-01-21 12:37:33 UTC
Created attachment 130597 [details]
Reworked dialog on gtk3

Removed the fix width (both 80 and 120px are bad) and added some code in order to set the width on runtime in respect to the longest text. Took me a while to find out why the buttons were squeezed- the code is uncommented now. And finally I unset the resizable flag for this dialog.

Changes are in https://gerrit.libreoffice.org/#/c/33370/1. Unfortunately there is something wrong with my code, it's my first real code contribution, which leads to a crash when the dialog is closed. Hope to get a solution from the reviewers.

PS: Learn more about how to submit patches at https://wiki.documentfoundation.org/Development/gerrit/SubmitPatch Your contribution is very welcome.
Comment 22 poma 2017-01-29 19:06:10 UTC
Heiko, thank you for the welcome.
Comment 23 poma 2017-01-29 19:07:43 UTC
Created attachment 130749 [details]
LO 5.3.0.3 - Boutons Rechercher

UI language French,
dialog "Rechercher & remplacer" (en. "Find & Replace"),
text "Rechercher le précédant" (en. "Find Previous") of the label within the button "backsearch",
and text "Rechercher le suivant" (en. "Find Next") of the label within the button "search",
need more horizontal space.

The relevant settings are:
- Font size: 12
- PPI (aka DPI): 200
- Desktop enviroment: Xfce
- Desktop theme: Darkgrey
- VCL UI interfaces: GTK+ 3

Tested with:
libreoffice --version
LibreOffice 5.3.0.3 30(Build:3)
patched with:
"tdf#105371, tdf#99456 Alignment in find & replace dialog"
https://gerrit.libreoffice.org/gitweb?p=core.git;a=commitdiff;h=8e96cc8
Comment 24 Heiko Tietze 2017-01-29 20:32:14 UTC
(In reply to poma from comment #23)
> - PPI (aka DPI): 200

Sounds like a WONTFIX. We would need to make the dialog larger for those unusual resolutions.
Comment 25 poma 2017-01-30 14:58:51 UTC
What a pity!
However, at least we tried to solve it.

Thank you for participating!
Comment 26 Heiko Tietze 2017-01-30 16:11:53 UTC
(In reply to poma from comment #25)
> Thank you for participating!

The patch for the labels is still pending. The dialog crashes LibO on my system when closed.
Comment 27 Commit Notification 2017-02-03 01:08:43 UTC
heiko tietze committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=789ed159fb03eef26c991f361380d0eb7b509cd9

tdf#105371, tdf#99456 Alignment in find & replace dialog

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 28 Heiko Tietze 2017-02-04 13:49:13 UTC
No crash anymore, either solved with other code or I just had to compile everything.
Comment 29 Commit Notification 2017-02-07 15:42:31 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6efdd843c831527b2880959746c302fca87c40db

Related: tdf#105371 we can do this with a sizegroup

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Heiko Tietze 2017-02-08 18:37:46 UTC Comment hidden (no-value)