Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toolbar icons are too large particularly in dark theme #8075

Closed
CAM-Gerlach opened this issue Oct 14, 2018 · 11 comments
Closed

Toolbar icons are too large particularly in dark theme #8075

CAM-Gerlach opened this issue Oct 14, 2018 · 11 comments

Comments

@CAM-Gerlach
Copy link
Member

Problem Description

Originally noticed on #8020 and part of the broader #8068 .

The size of the icons in the toolerbar, already a bit overly large (IMO) compared to other IDEs in the normal light theme, has become almost comical in the dark theme; this makes the chrome bigger, looks less polished, makes them not all fit on one line anymore while they fit quite comfortably before, and is a substantial UI inconsistency between themes. Therefore, it should be reduced to a common size at least as small as the current light theme.

Tested on Python 3.6.6, Windows 8.1 x64 and PyQt/Qt 5.9.6.

image

image

@jnsebgosselin
Copy link
Member

In addition, should we add an option to select the icon size (large or small)? This option is available in may software.

@CAM-Gerlach
Copy link
Member Author

@jnsebgosselin Sure, if you don't mind. "Large" can be the size we have currently in the light theme (32px) , while "Small" should be the standard size in most applications that still use toolbars (24 px); that's generally how I've seen it done anyway.

@jnsebgosselin
Copy link
Member

jnsebgosselin commented Oct 15, 2018

Maybe we should use the sizes provided by the pixel metrics to set the icon size instead of using hard coded values.

import sys
from PyQt5.QtCore import Qt
from PyQt5.QtWidgets import (QApplication, QStyle)
app = QApplication(sys.argv)
style = app.style()
print(style.pixelMetric(QStyle.PM_LargeIconSize))
print(style.pixelMetric(QStyle.PM_SmallIconSize))
print(style.pixelMetric(QStyle.PM_ToolBarIconSize))
app.exec_()

This results in 32, 16, 24 on my laptop with Windows 10.

@CAM-Gerlach
Copy link
Member Author

Great thinking! Yeah; ToolBarIconSize would correspond to the "Small" size for the toolbar that I suggest, while LargeIconSize is the current size in the light theme that I suggest for "large" (and the dark theme is even larger than that).

@ccordoba12
Copy link
Member

Maybe we should use the sizes provided by the pixel metrics to set the icon size instead of using hard coded values.

@jnsebgosselin, that's a really neat idea!! Could you prepare a PR for it? Thanks!

@jnsebgosselin
Copy link
Member

Maybe we should use the sizes provided by the pixel metrics to set the icon size instead of using hard coded values.

@jnsebgosselin, that's a really neat idea!! Could you prepare a PR for it? Thanks!

Sure thing.

What about the idea to add an option in the preferences where users can switch between 'large', 'normal', and 'small' size icons?

@ccordoba12
Copy link
Member

Sure, I'd like that too. Please go ahead with it.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Oct 20, 2018

FYI, icons in the toolbars of individual panels are also ~50% larger, which makes the minimum width of things like e.g. the outline explorer awkwardly large. We could probably just fix them at the size they are in the standard theme, since too much smaller and they'd be almost impossible to see, but too much larger and they'd constrain the minimum panel width.

image

image

@ccordoba12
Copy link
Member

This seems fixed in QDarkStyle 2.6.1. @CAM-Gerlach, please confirm and close if that's the case.

@jnsebgosselin, please open a new issue for your proposed changes, in case you still want to implement them.

@ccordoba12 ccordoba12 assigned CAM-Gerlach and unassigned dalthviz Nov 1, 2018
@CAM-Gerlach
Copy link
Member Author

Yup, looking good. Thanks @ccordoba12

image

@ccordoba12 ccordoba12 removed this from the v4.0beta2 milestone Nov 2, 2018
@dpizetta
Copy link
Contributor

dpizetta commented Nov 2, 2018

Oh, nice, I would ask for small icons :) more space for texting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants