Bug 435731 - Extra space between entries in Places panel after reverting icon size to "Small"
Summary: Extra space between entries in Places panel after reverting icon size to "Small"
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: panels: places (show other bugs)
Version: unspecified
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-14 15:39 UTC by Patrick Silva
Modified: 2021-04-23 14:22 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 21.04.1
Sentry Crash Report:


Attachments
small icons before step 1 (51.87 KB, image/png)
2021-04-14 15:39 UTC, Patrick Silva
Details
small icons after step 2 (51.89 KB, image/png)
2021-04-14 15:39 UTC, Patrick Silva
Details
built 21.04 branch (194.80 KB, image/png)
2021-04-23 14:16 UTC, Patrick Silva
Details
Dolphin 20.12.3 (233.44 KB, image/png)
2021-04-23 14:16 UTC, Patrick Silva
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2021-04-14 15:39:17 UTC
Created attachment 137605 [details]
small icons before step 1

STEPS TO REPRODUCE
1. right-click on an empty area of Places panel, hover over "Icon size",
change icon size from "Small" to "Medium"
2. revert icon size to "Small"
3. 

OBSERVED RESULT
compare the attached screenshots showing Dolphin with small icon size
before the step 1, and with small icon size after the step 2.

EXPECTED RESULT
after the step 2, Places panel should be the same seen in the screenshot
taken before step 1

SOFTWARE/OS VERSIONS
Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.21.80
KDE Frameworks Version: 5.82.0
Qt Version: 5.15.2
Graphics Platform: Wayland
Comment 1 Patrick Silva 2021-04-14 15:39:43 UTC
Created attachment 137606 [details]
small icons after step 2
Comment 2 Nate Graham 2021-04-14 15:57:03 UTC
What fonts are you using?
Comment 3 Patrick Silva 2021-04-14 16:32:17 UTC
Default Noto Sans.
Comment 4 Nate Graham 2021-04-14 18:55:18 UTC
Thanks. Cannot reproduce.
Comment 5 Patrick Silva 2021-04-14 19:35:24 UTC
Weird. I can even reproduce with Dolphin 21.04 RC and KDE Qt on Arch Linux.

Operating System: Arch Linux
KDE Plasma Version: 5.21.4
KDE Frameworks Version: 5.81.0
Qt Version: 5.15.2
Comment 6 Harald Sitter 2021-04-16 10:57:06 UTC
https://invent.kde.org/system/dolphin/-/commit/e9a39700fc004004b1ff231023e9d5333a2b8317

The padding is only applied IFF an explicit size has been set, and the way the settings behave is that when you go to medium and then small it will not remove the explicit size but set it to 16 (which honestly is a bit of a bug in of itself).

i.e. a "new" dolphinrc is not the same as a dolphinrc that changed the size at least once.

If you remove your dolphinrc and start from a clean state the bug should be reproducible. Once you switched back to Small the config contains:

[PlacesPanel]
IconSize=16

Which then triggers the PlacesView::setIconSize call which then inject artificial padding forever more.

The problem here really is that the commit I linked to is wrong. It shouldn't set the padding in the setIconSize, it should set it in the constructor. The padding never changes, there is no reason for it to be set over and over again to the same value.
Comment 7 Nate Graham 2021-04-16 16:30:44 UTC
Thanks for the investigation, Harald? I don't suppose you would be interested in submitting a little MR to fix it? :)
Comment 8 Harald Sitter 2021-04-16 17:41:08 UTC
I can, if you do the QA on it... it'd be the much more annoying bit :P
Comment 9 Nate Graham 2021-04-16 18:27:22 UTC
Will do. :)
Comment 10 Bug Janitor Service 2021-04-19 10:40:23 UTC
A possibly relevant merge request was started @ https://invent.kde.org/system/dolphin/-/merge_requests/197
Comment 11 Harald Sitter 2021-04-19 18:16:05 UTC
Git commit 56888a567fc741713b6c905aeed3842a7fa230c7 by Harald Sitter.
Committed on 19/04/2021 at 16:07.
Pushed by sitter into branch 'release/21.04'.

fix padding in places view

padding was only applied when the icon size was applied, the icon size
however is only applied when the user had set an explicit size. this
resulted in inconsistent spacing. by default no padding would be used if
the user had changed the icon size to medium and back to small it would
suddenly have padding.

to fix this, set padding unconditionally on construction and never touch
it again .

M  +5    -1    src/panels/places/placesview.cpp

https://invent.kde.org/system/dolphin/commit/56888a567fc741713b6c905aeed3842a7fa230c7
Comment 12 Patrick Silva 2021-04-23 14:15:24 UTC
I have built 21.04 branch on Arch Linux, now padding in Places panel with small icon size always (even with a new dolphinrc file) looks similar to what we can see in screenshot from comment 1. Is this intentional? I prefer the Places panel seen in the screenshot of Dolphin 20.12.3 but I can live with more padding if it is intentional.
Comment 13 Patrick Silva 2021-04-23 14:16:09 UTC
Created attachment 137831 [details]
built 21.04 branch
Comment 14 Patrick Silva 2021-04-23 14:16:42 UTC
Created attachment 137832 [details]
Dolphin 20.12.3
Comment 15 Harald Sitter 2021-04-23 14:18:18 UTC
(In reply to Patrick Silva from comment #12)
> Is this intentional?

That is my understanding from https://invent.kde.org/system/dolphin/-/merge_requests/78
Comment 16 Patrick Silva 2021-04-23 14:22:57 UTC
ok, thanks for your quick reply Harald.