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

Add style chooser in settings, refactor user style code #12180

Closed
wants to merge 3 commits into from

Conversation

ewancg
Copy link

@ewancg ewancg commented Sep 15, 2023

image
image
image
image

User style combo box is invisible when user styles are not in use.

@mbc07
Copy link
Contributor

mbc07 commented Sep 15, 2023

If we go that route and implement a dark/light style toggle, I think it would be more sensible to get rid of the separate User Style combo box and just merge both into this new "Style" combo box. For example, "System", "Light" and "Dark" would always be there (maybe add a "built-in" or similar indicator to the default light/dark styles), followed by any custom user style the end-user might have.

User doesn't have any custom style? "Style" combo box would display only "System", "Light" and "Dark". User has installed a custom style named "foo" and another named "bar", the "Style" combo box would now display "System", "Light", "Dark", "foo" and "bar", and so on...

@ewancg
Copy link
Author

ewancg commented Sep 15, 2023

Left my comments on merging the combo boxes in the Discord. Currently considering making the Style combobox act as a QStyle selector on Linux, and add explicit light/dark support for macOS (looks very possible at a glance)

Just to get it to pass CI runs, the following has to be done:

  • Fix lint errors
  • Add LoadLightStyle as a ref capture in LoadDarkStyle (maybe only if not Windows? errors on unused vars, maybe captures too)

Someone else can do this between now and later this afternoon, or I can do it then

@ewancg ewancg force-pushed the master branch 3 times, most recently from 555afa5 to da20bd2 Compare September 17, 2023 21:00
@AdmiralCurtiss
Copy link
Contributor

Closing this because #12272 was merged.

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