Skip to content

Conversation

kyleboyle
Copy link

@kyleboyle kyleboyle commented Apr 29, 2025

  • unifies icon style
  • adds specific icon colours for light and dark theme, no more contrast compromise
  • removes unused icons
  • due to lack of support for custom themes in the qt designer, the icons are not shown when editing a .ui file. (I may just not know how to correctly configure the designer for the icon themes though)

Example:

Kapture.2025-04-29.at.10.59.36.webm

@kyleboyle
Copy link
Author

Not sure why the build scripts are failing on the icon resource build

make: *** No rule to make target 'res/icons/dark/svg/done.svg', needed by 'qrc_icons.cpp'. Stop.

@aa5sh
Copy link

aa5sh commented Apr 29, 2025 via email

@aa5sh
Copy link

aa5sh commented Apr 29, 2025

image

@kyleboyle
Copy link
Author

thanks, seems git didn't handle all the renaming properly

@aa5sh
Copy link

aa5sh commented Apr 29, 2025

Yup that got it. Builds and looks fine now.

@foldynl
Copy link
Owner

foldynl commented May 3, 2025

Thanks. I have to find time to test it. Unfortunately, I recently noticed that QT under Windows automatically chooses a theme (Light/Dark) based on the Windows theme (Light/Dark). And I don't know how this change will behave under Windows.

@kyleboyle
Copy link
Author

Interesting. Well this type of icon theme is different from a colour palette so as long as the QLog colour theme state is in sync with what is instructed by windows then it should work as expected.

@kyleboyle
Copy link
Author

I see what you are saying about windows now. In windows the colour schemes in QLog equate to windows colours = light, QLog dark palette = dark. So if the user has their windows theme set as a dark mode type theme, then the icons for qlog light theme will not work well. I think I can handle this by choosing the icon set based on the brightness of the current palette's text colour.

@foldynl
Copy link
Owner

foldynl commented May 6, 2025

yes, that's exactly what I'm talking about. I found this out recently. I don't normally use dark mode on Windows, but recently I was showing someone QLog with enabled Windows dark mode and I was surprised that QLog was dark even though QLog switched light. It should be a feature of the latest QT 6.x. Just tested and Flatpak with 6.8 has the same behavior. So the question is whether the light/dark mode switch should be present when QT is 6.x. I think that it should be removed and left to the operating system settings.

@kyleboyle
Copy link
Author

I was thinking of this approach where we offer both options... convert the toggle button to a single icon button with an action menu that presents 3 options:

  • Theme: Native
  • Theme: QLog Light
  • Theme: QLog Dark

We can get the explicit palette values for the current qt default light theme and then rely on the os to provide the default palette for the native theme. Then the icons can be set depending on the state of the colours. I found an informative discussion about detecting light/dark here: https://stackoverflow.com/questions/75457687/detect-dark-application-style-theme-of-currently-used-desktop-in-qt

@kyleboyle
Copy link
Author

kyleboyle commented May 9, 2025

I have a proof of concept working:

Kapture.2025-05-08.at.23.04.55.webm

I need to clean it up, including the state saved in activity settings etc

kyleboyle added 2 commits May 12, 2025 16:40
…icons

# Conflicts:
#	ui/BandmapWidget.ui
#	ui/QSODetailDialog.ui
@kyleboyle
Copy link
Author

kyleboyle commented May 12, 2025

Changes complete. Would be best to get it tested on other platforms. It works as in the above screen recording where QLog provides a light and dark theme and the user can also use the default native style. The icon theme used is based on if the text color should be dark or light.

@foldynl
Copy link
Owner

foldynl commented Jun 1, 2025

@kyleboyle sorry I'm getting to this now. I've been fully focused on the Upload/Download rework and the big change in QLog paramaters.

I tried your change and noticed a few side effects that are caused by your change.

  1. I don't know what could be causing this, maybe using a UI theme caused it, but on Linux the icons from the menu disappeared.
    After Your change
    image

Before change
image

  1. I'm not sure if unifying the icons is the right approach. Each Linux distribution has slightly different icons and themes, and users are used to a certain style (Button Save/Load etc.). If we replace the icons with unified ones, it might be confusing for some users, as other software shows the Save icon one way, but QLog uses a different one.

  2. As you wrote, Working with the UI in Qt Designer is very inconvenient after the change, because the icons are not visible, and what's worse, Qt Creator modifies it in such a way that it stops working every time I make a change.

I don't know how to solve the points above right now, but at the very least, point 1 is a obstacle to merge it. Point 3 is something that's inconvenient for development, but it doesn't affect the user. But to be honest, WYSIWYG design is a big advantage of QT Designer/Creator.

Therefore, I think we should, for now, use only the theme-switching logic from this PR without icon changes. Given the finding that Qt automatically switches themes under Qt 6.x, this change will be necessary.

@kyleboyle
Copy link
Author

Makes sense. I think the icon experience we currently have is not ideal because on macos there aren't menu icons and many of the button icons also don't show. In windows most of the menu icons also don't show. On linux the icons are mixed between system theme and QLog provided. In any case, this PR is my attempt to show a better way to display icons consistently cross-platform and cross-theme. The missing menu icons could be fixed by defining our own icons instead of relying on system theme.
Cheers
Kyle

@aa5sh
Copy link

aa5sh commented Jul 7, 2025

Could the icons be pulled out from the UI and put in code so it could defined with a fallback. So on OS's like MacOS where the icons don't exist they could use ones provided if not it would use the system.

QIcon icon = QIcon::fromTheme("application-certificate", QIcon(":/icons/application-certificate.png"));
QAction *certAction = new QAction(icon, tr("Certificate"), this);

@kyleboyle
Copy link
Author

My next step for this change is to separate out the theme selection and icon changes as suggested by Ladislav, hoping to get the theme changes themselves in. Then the next step is to tinker with the icon part. I didn't explore if it was possible to extend existing themes, etc. I would hate to have to programmatically create an icon object for every place there is an icon used.

@kyleboyle
Copy link
Author

I'm closing for now. see #718 and I will continue to ruminate on the icon stuff.

@kyleboyle kyleboyle closed this Jul 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants