DevHeads.net

Review Request 108442: [High-dpi issues] Fix KIconButton initial icon size and its occurence in KPropertiesDialog

Review request for kdelibs.

Description
The KIconButton and the other occurences assume that the icon size for desktop icons is always 48x48. This assumption is wrong.
This patch makes KPropertiesDialog use the proper IconSize.

There are other places that need fixing too (eg. Dolphin's Place edit dialog or KMenuEdit) which I will fix later as well.

So, with KDE Frameworks at the horizon and kdelibs frozen, does this mean, when I am re-writing the KIconDialog to be more userfriendly, use an UI file, introduce new strings, etc this cannot go into master but only frameworks branch?

Diffs
kio/kfile/kicondialog.cpp b7d646f
kio/kfile/kpropertiesdialog.cpp 223ac7c

Diff: <a href="http://git.reviewboard.kde.org/r/108442/diff/" title="http://git.reviewboard.kde.org/r/108442/diff/">http://git.reviewboard.kde.org/r/108442/diff/</a>

Testing
Yup, you won't notice any difference with default settings but with higher icon size and font scales perfectly and looks good. See screenshot.

File Attachments
KPropertiesDialog with Retina settings
<a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png" title="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png">http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutto...</a>

Thanks,

Kai Uwe Broulik

Comments

Re: Review Request 108442: [High-dpi issues] Fix KIconButton ini

By Kai Uwe Broulik at 05/15/2013 - 15:37

(Updated May 15, 2013, 7:37 p.m.)

Review request for kdelibs.

Changes
Use screen dpi to calculate the button size

Description
The KIconButton and the other occurences assume that the icon size for desktop icons is always 48x48. This assumption is wrong.
This patch makes KPropertiesDialog use the proper IconSize.

There are other places that need fixing too (eg. Dolphin's Place edit dialog or KMenuEdit) which I will fix later as well.

So, with KDE Frameworks at the horizon and kdelibs frozen, does this mean, when I am re-writing the KIconDialog to be more userfriendly, use an UI file, introduce new strings, etc this cannot go into master but only frameworks branch?

Diffs (updated)
kio/kfile/kicondialog.cpp 73a7449
kio/kfile/kpropertiesdialog.cpp b4cd8ee

Diff: <a href="http://git.reviewboard.kde.org/r/108442/diff/" title="http://git.reviewboard.kde.org/r/108442/diff/">http://git.reviewboard.kde.org/r/108442/diff/</a>

Testing
Yup, you won't notice any difference with default settings but with higher icon size and font scales perfectly and looks good. See screenshot.

File Attachments
KPropertiesDialog with Retina settings
<a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png" title="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png">http://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutto...</a>

Thanks,

Kai Uwe Broulik

Re: Review Request 108442: [High-dpi issues] Fix KIconButton ini

By Kai Uwe Broulik at 12/15/2015 - 17:52

(Updated Dez. 15, 2015, 9:52 nachm.)

Status
This change has been discarded.

Review request for kdelibs.

Repository: kdelibs

Description
The KIconButton and the other occurences assume that the icon size for desktop icons is always 48x48. This assumption is wrong.
This patch makes KPropertiesDialog use the proper IconSize.

There are other places that need fixing too (eg. Dolphin's Place edit dialog or KMenuEdit) which I will fix later as well.

So, with KDE Frameworks at the horizon and kdelibs frozen, does this mean, when I am re-writing the KIconDialog to be more userfriendly, use an UI file, introduce new strings, etc this cannot go into master but only frameworks branch?

Diffs
kio/kfile/kicondialog.cpp 73a7449
kio/kfile/kpropertiesdialog.cpp b4cd8ee

Diff: <a href="https://git.reviewboard.kde.org/r/108442/diff/" title="https://git.reviewboard.kde.org/r/108442/diff/">https://git.reviewboard.kde.org/r/108442/diff/</a>

Testing
Yup, you won't notice any difference with default settings but with higher icon size and font scales perfectly and looks good. See screenshot.

File Attachments
KPropertiesDialog with Retina settings
<a href="https://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png" title="https://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutton.png">https://git.reviewboard.kde.org/media/uploaded/files/2013/01/16/iconbutt...</a>

Thanks,

Kai Uwe Broulik

Re: Review Request 108442: [High-dpi issues] Fix KIconButton ini

By =?utf-8?Q?Thoma... at 06/01/2013 - 14:27

kio/kfile/kpropertiesdialog.cpp
<http://git.reviewboard.kde.org/r/108442/#comment24810>

18*dpiRatio + 2*iconButton->style()... ?
The code looks suspisious enough, though (the button should have a sanme resizePolicy and adjust to the loaded icon automatically, yesno?) *shrug*

- Thomas Lübking

On May 15, 2013, 7:37 p.m., Kai Uwe Broulik wrote:

Re: Review Request 108442: [High-dpi issues] Fix KIconButton ini

By =?utf-8?Q?Thoma... at 02/18/2013 - 15:44

kio/kfile/kicondialog.cpp
<http://git.reviewboard.kde.org/r/108442/#comment20724>

Similar as with the other RR: if i set the desktop icon size to 256 (cause i like huge icons on my desk or whatever) i'll get HUUGE Ui elements, possibly dropping the GUI out of screen boundaries.

I'd go for a dpi aware pick of 48x48 as well.

- Thomas Lübking

On Jan. 16, 2013, 6:18 p.m., Kai Uwe Broulik wrote:

Re: Review Request 108442: [High-dpi issues] Fix KIconButton ini

By Kai Uwe Broulik at 02/18/2013 - 12:45

Ping?

- Kai Uwe Broulik

On Jan. 16, 2013, 6:18 p.m., Kai Uwe Broulik wrote: