Bug 241277

Summary: the dialog for hiding entries in the systray has a bad layout
Product: plasma4 Reporter: Christian Trippe <christiandehne>
Component: widget-systemtrayAssignee: Jan Gerrit Marker <jangerrit>
Status: RESOLVED FIXED    
Severity: normal CC: aseigo, ashl1future, aspotashev, asraniel, caionnew, jangerrit
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: openSUSE RPMs   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Screenshot of the dialog
Screenshot which probably after first commiting to resolve this bug
There no difference between 2 values in visibility columns whan try to change
Proposed patch
Screenshot 1 of Variant B
Screenshot 2 of Variant B

Description Christian Trippe 2010-06-10 10:15:52 UTC
Created attachment 47847 [details]
Screenshot of the dialog

Version:           unspecified (using Devel) 
OS:                Linux

The dialog where one can configure which entries of the systray should be hidden has a bad layout, a screenshot is attached.

The size of the columns for the name and the visibilty are to small while the one for shortcuts is quite big.

Qt: 4.6.2
KDE: 4.4.85 (KDE 4.4.85 (KDE 4.5 Beta2)) "release 3"

Reproducible: Always
Comment 1 Alexey Shildyakov 2011-02-11 18:22:11 UTC
Still presents in KDE 4.6.0
Comment 3 Alexey Shildyakov 2011-08-05 16:36:02 UTC
Created attachment 62578 [details]
Screenshot which probably after first commiting to resolve this bug

Think that's bad idea to use QHeaderView::ResizeToContents, because "The size cannot be changed by the user or programmatically". It's wrong deny user from change column width. As I understand, icons() is just only application name column, so for me it'll be as in attached screenshot. It's bad.
Comment 4 Alexey Shildyakov 2011-08-05 16:36:23 UTC
Ok. Lets thinking what user want to see.
1. User should understand the application in the row.
2. User should understand current visibility and what visibility he can choose.
3. User should see the key bindings.

The problems:
1. The name length of many apps is small. But for system KDE apps the name length may be long (as I can see) and we don't know how long it will be (depend on i18n)
2. The right part of text shown in the button is truncated in normal mode, and the middle part is truncated when choose the value. For me it's bad because there are no difference in 2 values. (in next screenshot)
3. By default, I don't have key bindings. But if I have at least one key bindings I should know what it is for and what the binding is.

I was thinking a lot today about this problem. First, I think that we should set the width of the first row (where apps) to show the most of apps (maybe 80% or so). But I don't understand the level of width from what we should truncate the text. Then I just think about the main problems and see the list described above. And what I have thunk:

Variation A:
1. We should statically set width of the first column to the value, when the most of apps will be shown at whole.
2. Then dynamically set the width of the second column (depend on the localized text lentgh).
3. And dynamically set the width of the last column. 
The problem may occur, when the horizontal scrool bar added and the third column won't be shown. When user scrool right to change key binding he may not see the application name.

Variant B:
Change the columns order to be: visibility, apps name, key binding.
1. Dynamically set visibility column width depending on its content (the longerst of 3 values)
2. Set apps name column width as (100%_of_visible_field - visibility_width - 50px)
50 px is for the third column. In that case the horizontal scrool always would be presented but 50 px help the user understand that in right something is hide.
3. Dynamically set key bindings column width depending on its content (the longest of any key binings row) (should not be greater then 100%-apps_name_width by default)

I like the Variant B because:
1. The whole info is visible about visibility and key bindings and the most important part of app name (probably the whole text).
2. User understand about something at the right (it will be key bindings) and user always see apps name when edit visibility or key bindings.
3. Visibility - the important property is shown at whole by default.
4. The almost application name is shown.
5. User can manually change columns width.

What Aaron think about this?
Comment 5 Alexey Shildyakov 2011-08-05 16:37:05 UTC
Created attachment 62579 [details]
There no difference between 2 values in visibility columns whan try to change
Comment 6 Jan Gerrit Marker 2011-08-07 09:19:04 UTC
I read your comments and agree with you that my commit doesn't fix the bug properly. Currently I'm working on a solution but sadly the Qt classes do not provide very convenient methods for resizing the columns. Maybe you want to have a look at the patch I attach and comment it (I played around with the functions Qt provides, maybe it's the right track - it's similar to your solution A). I added tooltips which can show the name of the application if it is shortened. Solution B seems pretty good but sadly there's no way to set the column size in percent.
Thanks for thinking about this.
Comment 7 Jan Gerrit Marker 2011-08-07 09:20:20 UTC
Created attachment 62633 [details]
Proposed patch
Comment 8 Alexey Shildyakov 2011-08-10 09:19:51 UTC
Created attachment 62720 [details]
Screenshot 1 of Variant B
Comment 9 Alexey Shildyakov 2011-08-10 09:22:32 UTC
Created attachment 62721 [details]
Screenshot 2 of Variant B

I attached 2 screenshots of Variant B I'm talking about. Think this is the best way. I explain below, how it could be implemented. I don't have development environment, so I couldn't to make and test myself patch, so I hope to Jan Gerrit Marker.
Comment 10 Alexey Shildyakov 2011-08-10 09:48:38 UTC
Variant B is more sophistigated, but more better than others.
Implementation:
1. Change columns (visibility, appname, key bindings). Check if other code works properly with this changes and change if needed.
2. a) For the first column (visibility) use QHeaderView::ResizeToContents. Check the test cases 2, 4, 5.
   b) In case the a) has incorrect behavior, should use QWidget::fontMetrics()/QFontMetrics to understand the longest of 3 values. Use QHeaderView::Interactive and use QHeaderView::resizeSection to resize the section to appropriate value.
3. a) Create new method with the contents: resize the second column (appname) to be  m_autoHideUi.icons->header()->offset() - m_autoHideUi.icons->header()->sectionSize(0) - 50px.
   b) Check the test case 1 - how the third column is shown in default state. I'm not sure if absolute value (50px) is good idea because think in different screen resolution real visible width in default view will be different.
   c) If test case 1 will fail then should be think how to use the relative value.
4. For the third column (keybindings) use QHeaderView::ResizeToContents and check the test cases 2, 3.

Test cases (must be tested):
1. Change screen resolution.
  a) Open the systemtray widget settings and don't close. Then change resolution. Check it.
  b) Look the systemtray widget settings in one resolution, test it, then close. Change resolution, then open systemtray widget settings, look and test it again.
2. Change systemtray widget settings window width and look how it will be. Test the minimum width case and long width case.
3. And more key bindings (the maximum and the longest that possible) and test how column will be streched.
4. Click on visibility and choose every of 3 values. Check if the text is showed properly.
5. Change l10n KDE language and see if the text in all 4 testcases above is correct.

All test cases should be applied before finish patching.
Comment 11 Alexey Shildyakov 2011-08-10 15:57:34 UTC
Of course test case 2 to be worked appropriate behavior (slot) should be implemented.
Comment 12 Beat Wolf 2011-11-21 22:52:11 UTC
what happend to this patch? please submit it to https://git.reviewboard.kde.org or else it will get lost here.
Comment 13 Aaron J. Seigo 2011-11-22 08:00:06 UTC
the behaviour in master is just fine. no need for increasingly odd layouts in that window.