Bug 390771 - When default font size is increased by 1pt icons in places panel are out of alignment with corresponding label
Summary: When default font size is increased by 1pt icons in places panel are out of a...
Alias: None
Product: dolphin
Classification: Applications
Component: panels: places (show other bugs)
Version: 17.08.3
Platform: Debian testing Linux
: NOR minor (vote)
Target Milestone: ---
Assignee: Scott Harvey
Keywords: junior-jobs
Depends on:
Reported: 2018-02-20 09:50 UTC by Andrea Panontin
Modified: 2018-03-29 22:30 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:

Incorrect vertical alignment (20.38 KB, image/png)
2018-02-20 09:50 UTC, Andrea Panontin
Top-aligned icons in Places panel (11.96 KB, image/png)
2018-03-22 17:14 UTC, Scott Harvey
Top-aligned icons in file listing (14.92 KB, image/png)
2018-03-22 21:36 UTC, Scott Harvey

Note You need to log in before you can comment on or make changes to this bug.
Description Andrea Panontin 2018-02-20 09:50:20 UTC
Created attachment 110836 [details]
Incorrect vertical alignment

I've increased the default font size by 1pt in system settings and, as a result, in dolphin's places panel the icons are not vertically aligned with the corresponding label (as you can see in the screenshot). I've not yet tested it with the latest plasma (and dolphin) since I'm on debian testing and it is still not available
Comment 1 Patrick Silva 2018-02-20 13:17:16 UTC
Confirmed in plasma 5.12.1, Arch Linux.
Comment 2 Patrick Silva 2018-02-20 15:59:30 UTC
(In reply to Dr. Chapatin from comment #1)
> Confirmed in plasma 5.12.1, Arch Linux.

dolphin 17.12.2.
Comment 3 Scott Harvey 2018-03-22 17:14:04 UTC
Created attachment 111557 [details]
Top-aligned icons in Places panel
Comment 4 Scott Harvey 2018-03-22 17:16:18 UTC
This can be recreated any time the text size is larger than the icon size - the icons are top-aligned in the menu entry rows. In my attachment, I set the icon size to 22x22 and cranked up my system font size to 18pt. 

I'll have a look at fixing this.
Comment 5 Scott Harvey 2018-03-22 17:17:40 UTC
CC: Nate Graham, interface and usability guru.
Comment 6 Nate Graham 2018-03-22 17:19:44 UTC
(In reply to Scott Harvey from comment #5)
> CC: Nate Graham, interface and usability guru.

Hah! Hardly; just an interested amateur, really.

Looks like we just need to make the icons centered rather than top-aligned. Thanks for having a go at this, Scott!
Comment 7 Scott Harvey 2018-03-22 21:35:12 UTC
Hm, this might be a bit more complicated than I first thought. You can also cause the same problem in the regular file view. 

Set the view to "Compact" (icon & 1 row of text) mode, and slide the icon-size slider all the way to minimum. Bump up your text size if necessary, and you'll see the same top-aligned icon. I'll add another screenshot.

It looks like the "Places" panel uses the same list-display library as the main file view window. 

In other words, I have to be excruciatingly careful with any changes I make. Breaking all of Dolphin would probably be frowned upon.
Comment 8 Scott Harvey 2018-03-22 21:36:00 UTC
Created attachment 111565 [details]
Top-aligned icons in file listing
Comment 9 Nate Graham 2018-03-29 22:30:03 UTC
Git commit e0f7fe87bdc2e32778cd6ce997b59b883cb245b8 by Nathaniel Graham, on behalf of Scott Harvey.
Committed on 29/03/2018 at 22:29.
Pushed by ngraham into branch 'master'.

Fix alignment of icons in Places panel and Compact view mode

Adjust calculation of icon pixmap Y value; now based off center of text label bounding rectangle. Previously, icons appeared top-aligned when text size was larger than icon size.

Centering is done by obtaining the center point of the text frame (`m_textRect.center().y()`) and setting the top `Y` point of the icon to one-half of the scaled icon height (`m_scaled_PixmapSize.height()`)  Division is done by `2.0`, to ensure calculations are done with floating-point math, in keeping with `QPointF`, which returns floating-point values.

Also includes an adjustment named `midlineShift` (contributed by @zzag), which takes into account the font's midline in respect to the center of the text frame. Certain fonts (i.e. Noto Sans) can have a slightly offset midline, resulting in imperfect alignment of the icon. This small adjustment resolves that potential issue.

See before and after screenshots.
Before, showing misalignment (with and without debugging wireframes)

After, showing correction

Test Plan:
-- Compile Dolphin with patch
-- Increase system font size by several points (i.e. 15pt)
-- Check that Places panel icons remain centered with the text label
-- Select Compact View mode
-- Adjust icon size slider to minimum
-- Ensure that icons also remain centered in file listing
-- Check several combinations of icon size & font size to ensure centering is consistent

Reviewers: #dolphin, ngraham, cfeck, elvisangelaccio

Reviewed By: #dolphin, ngraham, elvisangelaccio

Subscribers: zzag, elvisangelaccio, #dolphin

Differential Revision: https://phabricator.kde.org/D11650

M  +11   -5    src/kitemviews/kstandarditemlistwidget.cpp