(icon view) Text width relative to icon size
AbandonedPublic

Authored by elvisangelaccio on May 19 2018, 7:44 PM.

Details

Reviewers
skaal
Group Reviewers
Dolphin
VDG
Summary

This changes the calculation for text width to rely on icon size. Also changes min. width to 64px (up from 48px).

Some comparisons:
width: medium
width: large

Patched version also allows for tighter icon grid when using small icons with >=medium text width.

Diff Detail

Repository
R318 Dolphin
Lint
Lint Skipped
Unit
Unit Tests Skipped
skaal created this revision.May 19 2018, 7:44 PM
Restricted Application added a subscriber: kfm-devel. ยท View Herald TranscriptMay 19 2018, 7:44 PM
skaal requested review of this revision.May 19 2018, 7:44 PM
xyquadrat added a subscriber: xyquadrat.

You forgot to specify any reviewers, so I added them for you. +1 from my side, removing code while improving the user experience is always a good thing :)

broulik added a reviewer: VDG.May 22 2018, 9:02 AM
alex-l added a subscriber: alex-l.May 22 2018, 9:06 AM

Very nice looking ๐Ÿ‘๐Ÿป

ngraham added a subscriber: ngraham.EditedMay 22 2018, 1:05 PM

In the abstract, I think this makes sense, but in testing, I ran into two issues:

  • This has the effect of making the default label width/grid spacing significantly narrower
  • Because the label width doesn't take into account the font size (currently as well as with your patch), people who increase the standard font size to 11 or even 12 pt are going to be irritated when more of their file names are split across lines because their labels are narrower than they used to be. Crank the font size up to 15 and the result is... interesting. It's a pre-existing issue, of course, but it becomes more urgent to fix with this patch.

For item #2, adding the font metrics into the calculation should suffice. For Item #1, Perhaps we should normalize the new calculation such that it's roughly the same or only slightly smaller for the standard Medium label width. I don't hear a lot of users complaining that the grid is too loose right now.

abetts added a subscriber: abetts.May 22 2018, 2:26 PM

Screenshot please? :D

There are videos linked in the summary.

Can we please move this patch into Gitlab over at https://invent.kde.org/system/dolphin and continue the work? :)

elvisangelaccio commandeered this revision.Nov 6 2020, 12:34 AM
elvisangelaccio abandoned this revision.
elvisangelaccio added a reviewer: skaal.