D11135: Scale font while zooming

Nathaniel Graham noreply at phabricator.kde.org
Wed Mar 7 17:04:32 GMT 2018


ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  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.

INLINE COMMENTS

> dolphinitemlistview.cpp:69
>      }
> +    constexpr auto iconToTextScalingFactor = 0.625;
> +    settings.setFontSize(iconToTextScalingFactor * iconSize);

Magic number

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D11135

To: rominf, #dolphin, ngraham
Cc: ngraham, #dolphin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180307/a511685b/attachment.htm>


More information about the kfm-devel mailing list