[WIP] Scale font while zooming
AbandonedPublic

Authored by rominf on Mar 7 2018, 4:47 PM.

Details

Reviewers
ngraham
Group Reviewers
Dolphin
Summary

Now zooming affects only icon sizes. This commit scales filename font size too.
BUG: 366367

Diff Detail

Repository
R318 Dolphin
Branch
zoom-font
Lint
No Linters Available
Unit
No Unit Test Coverage
rominf requested review of this revision.Mar 7 2018, 4:47 PM
rominf created this revision.
rominf edited the summary of this revision. (Show Details)Mar 7 2018, 4:51 PM
ngraham requested changes to this revision.Mar 7 2018, 5:04 PM
ngraham added a subscriber: ngraham.

Hmm, in that screenshot, the text is way too big for the size of the icons. It doesn't look good at all. Testing it out for myself, I was able to trivially break it after 10 seconds of testing:

  • The text enlarges permanently as soon as you adjust the zoom slider, and is herafter vastly too large for all icon sizes
  • In icons view mode, the horizontal space available for text doesn't increase, so at larger font sizes, each line only has a few characters on it
  • The selection marker (the blue horizontal line) doesn't adjust properly, so it appears inside the text, rather than always below it

Was this tested at all?

I agree with the change in principle as long as the effect is subtle, but this patch is not acceptable. Prominent visual changes like this need a ton of testing before even submitting the patch, unless it's going to be a [WIP] patch. This needs a lot more testing and visual polish before we can consider it.

src/views/dolphinitemlistview.cpp
69

Magic number

This revision now requires changes to proceed.Mar 7 2018, 5:04 PM
rominf retitled this revision from Scale font while zooming to [WIP] Scale font while zooming.Mar 7 2018, 5:07 PM
rominf added a comment.Mar 7 2018, 5:12 PM

OK, it was RFC, I wanted to know whether I'm doing the right thing or not at all. Sorry for not specifying this in the first place.

About magic number and "text is way too big for the size of the icons". I calculated this number simply by using 2 numbers: initial icon size and initial font size. 10/16=0.625. In my opinion, it looks OK.

Anyway, should I continue working in the same direction, or not?

ngraham added a comment.EditedMar 7 2018, 5:20 PM

I think this could potentially work with a lot more design and visual polish. Right now we're not even to the point where we can discuss whether or not it looks good; we're still in "it's too buggy to actually use" territory, so we have to fix those bugs first. I outlined three such bugs in my initial comment.

rkflx added a subscriber: rkflx.EditedMar 8 2018, 12:15 AM

I'm not convinced at all the text size should change. This would make it impossible to have normal-sized text and choose the item size independently, in particular (but not only!) for previews.

What's the use case for having larger text? As far as I understand, the zooming feature is for previews and for having larger click targets in the viewport. Text size should be set in System Settings or Dolphin's config.

Neither Windows Explorer, nor macOS Finder combine zooming icons and text. To me it seems the reporter does not know about KMag or KWin's magnifying glass, or wants easier access to changing the font size. However, that niche use case should not break the feature for nearly everyone else.

rominf abandoned this revision.Mar 8 2018, 9:26 AM

OK, @rkflx, you convinced me. Moreover, Nautilus acts the same way (text size is always the same).

When I made a positive comment in the bug report, what I had in mind was that the text would very selectively and conditionally become a bit larger when the icon size is huge. Because right now, 10pt font with 256pt icons just looks silly:

Whereas increasing it to 15pt looks a lot more appropriate IMHO:

But I think it would have to be hand-tuned, rather than just implementing a simple linear scaling factor like this patch does.

If everybody thinks I'm smoking crack and nobody else wants to do it, we should WONTFIX the bug.

rominf added a comment.Mar 8 2018, 3:51 PM

When I made a positive comment in the bug report, what I had in mind was that the text would very selectively and conditionally become a bit larger when the icon size is huge. Because right now, 10pt font with 256pt icons just looks silly.
Whereas increasing it to 15pt looks a lot more appropriate IMHO.
But I think it would have to be hand-tuned, rather than just implementing a simple linear scaling factor like this patch does.

+1. But we should not forget about user configured font size. I.e. if the user configured to use font size 14, then we should scale it to 21 (but not 15) for 256pt icons.

When I made a positive comment in the bug report, what I had in mind was that the text would very selectively and conditionally become a bit larger when the icon size is huge. Because right now, 10pt font with 256pt icons just looks silly.
Whereas increasing it to 15pt looks a lot more appropriate IMHO.
But I think it would have to be hand-tuned, rather than just implementing a simple linear scaling factor like this patch does.

+1. But we should not forget about user configured font size. I.e. if the user configured to use font size 14, then we should scale it to 21 (but not 15) for 256pt icons.

Right, exactly! If you re-did this patch along those lines, I think we could take a serious look at it.

rkflx added a comment.EditedMar 8 2018, 7:29 PM

@ngraham What you are essentially saying is that you find the font size in Gwenview's Browse mode (which shows exactly those "large" type of items like in Dolphin, i.e. thumbnails) too small, and even if the user set it to a normal size you would just increase it a bit, making it look inconsistent and frustrating the user.

We can already set the icon size as well as the text size. What you are doing here (under the name of "increase the text size") is to combine both into a dynamic slider. Would you also suggest to increase the size of all UI elements when someone maximizes a window? Or attach a slider to the menubar so users can quickly increase that size?

This is not a zoom slider like on a web page, where you temporarily increase the size. The slider is used to permanently select a size of the icons/previews, and if there is no way to combine that setting with having a normal/permanent text size, that's bad. Where do you make the cut for turning off the feature? For 128px? For 64px? What about HiDPI, or custom default font sizes? I don't think your plan will work, and it's better to have it looking a bit odd like in your screenshot, than to create problems in all other cases.

I.e. if the user configured to use font size 14, then we should scale it to 21 (but not 15) for 256pt icons.

If I configure a size for text and another size for icons, I want to get exactly that, and not something different. (Also note that if Use common properties for all folders is selected, the slider and the config entry for the default icon size are linked together.)


Anyway that's just my take on it, better ask more of Dolphin's maintainers about it before putting too much work into a patch.

cfeck added a subscriber: cfeck.Mar 8 2018, 9:19 PM

My two cents: You can configure both icon size and text size, for each view mode separately. This configuration can be considered the permanent state.

Now the question is, why would one temporarily increase the icon or the text size? The only reason I can come up with is when you are looking at a folder of image, video, or text thumbnails, and those reveal more information with larger icon sizes. To me that is exactly the use case for the zoom slider.

rkflx added a comment.Mar 8 2018, 9:23 PM

My two cents: You can configure both icon size and text size, for each view mode separately. This configuration can be considered the permanent state.

Now the question is, why would one temporarily increase the icon or the text size? The only reason I can come up with is when you are looking at a folder of image, video, or text thumbnails, and those reveal more information with larger icon sizes. To me that is exactly the use case for the zoom slider.

@cfeck So to be clear: Increasing the text size does not reveal more information, and thus it should not be increased in sync?

cfeck added a comment.Mar 8 2018, 10:12 PM

Right, sorry if this wasn't clear.

If I configure a size for text and another size for icons, I want to get exactly that, and not something different. (Also note that if Use common properties for all folders is selected, the slider and the config entry for the default icon size are linked together.)

Then you'd better go complain to Plasma and QQC2-desktop-style and Kirigami, because they all use a Heading control that essentially does exactly what this patch adds to Dolphin: provides a text label whose font size is some multiplier of the size you actually chose. It is commonly used for non-headings, as its smallest size "5" is equal to the size you set. 4 is a little bigger, 3 is a bit bigger, and so on, until you get to 1, which is an actual header-sized text label.

The point here is to allow standardized font sizes to be played with according to the designer's intent. Sometimes a slightly larger text size looks better in some situations. That's really all it is.

rkflx added a comment.Mar 13 2018, 1:01 AM

If I configure a size for text and another size for icons, I want to get exactly that, and not something different. (Also note that if Use common properties for all folders is selected, the slider and the config entry for the default icon size are linked together.)

Then you'd better go complain to Plasma and QQC2-desktop-style and Kirigami, because they all use a Heading control that essentially does exactly what this patch adds to Dolphin: provides a text label whose font size is some multiplier of the size you actually chose. It is commonly used for non-headings, as its smallest size "5" is equal to the size you set. 4 is a little bigger, 3 is a bit bigger, and so on, until you get to 1, which is an actual header-sized text label.

The point here is to allow standardized font sizes to be played with according to the designer's intent. Sometimes a slightly larger text size looks better in some situations. That's really all it is.

The different sizes you talk about conceptually correspond to the different static sizes users can set in the fonts KCM (e.g. title, normal text, small text etc.), only that they follow a more modern approach with sizing, rooted in desktop publishing and web design. There's nothing wrong with that.

What tickled me was the proposal to dynamically bind the font size to a zoom slider controlling the icon size. That's not something I've seen elsewhere so far, not even in Kirigami apps.