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

Dark Mode adjustments #91

Closed
pavlidvg opened this issue Jan 22, 2023 · 7 comments
Closed

Dark Mode adjustments #91

pavlidvg opened this issue Jan 22, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@pavlidvg
Copy link

I've been working with some adjustments in the dark theme of the downloader. I've done some slight color adjustments to improve readability (for example links are very hard to read on current dark mode). Only slight adjustments on some niche components, nothing too extreme since I already like the palette a lot.

I'd also like to add the ability to switch back to the default theme through the combo box. It annoys me to have to restart for light theme, and I guess it would be helpful if someday theme selection moves out of the 'experimental' menus.

I am already working on this. Would you be interested in a future pull request once I'm sure it's ready?

@dhtdht020
Copy link
Owner

dhtdht020 commented Jan 22, 2023

That would be great.

I am not sure of the ideal approach to move themes out of experimental options yet.
The commonly used frozen build for Windows does not provide convenient access to the themes directory.
They need to be convenient for the user to set up, so maybe something like a file picker and a settings-stored list of recently applied themes would work, in addition to the built in options like default and dark. Additionally, the program should keep the active theme in settings and always use it on startup.

This can potentially implemented through menus only, I think the dialog is out of place and can be dropped altogether. Something like this could work:
image

Want to hear your thoughts on this, a pull request in the future would be highly appreciated.

(Note that OSCDL does not really have a light theme, in macOS and Linux the default theme palette takes over, but Windows lacks a dark theme for its native controls. Potential windows dark mode support for Qt is tracked at https://bugreports.qt.io/browse/QTBUG-72028)

@dhtdht020 dhtdht020 added the enhancement New feature or request label Jan 22, 2023
@pavlidvg
Copy link
Author

I really like it from a UX perspective, and the file dialog would make more sense with the menu you showed. My current implementation though only switches through the existing QMessage, adds a 'System Default' Option, and toggles between them, while saving the User preference for the next startup.

Would you prefer if also implemented the actions shown in your picture? I just didn't want to take the liberty of removing theme option from 'Experimental' without consulting you first.

@dhtdht020
Copy link
Owner

That's fine too, UX is something I can do later on, though it will be highly appreciated if you do.

@pavlidvg
Copy link
Author

@dhtdht020
I sent a pull request with some of the things we discussed being implemented. I do not considered this issue closed personally, and would like to implement the features discussed with the UI you proposed. Just review the code and tell me if the backend side of things is clean enough for your project.

A few notes on the Pull request I forgot to mention. The 'Settings' file I added seemed like the cleanest way to save user preferences. For now it is just a theme saver, but I have used a similar json file in other projects and think that it's an easy and effective to save user preferences

@pavlidvg
Copy link
Author

@dhtdht020 Hello again and sorry for the tag.

I am now finishing up implementing the theme selection with an action menu like that you showed in the screenshot. It'll probably be ready in the next 24 hours (Still working out a few details with code formatting and user friendly File dialogs)

Would you want me to commit it as a change to Pull Request #92 or a next one? Also, should I take the liberty of removing the experimental feature or do I keep it in? It should still work right now and even keep adding the user themes, although it might need further testing

@dhtdht020
Copy link
Owner

You can keep it for now, and sure, commit it to PR #92.

pavlidvg added a commit to pavlidvg/osc-dl that referenced this issue Jan 29, 2023
pavlidvg added a commit to pavlidvg/osc-dl that referenced this issue Jan 29, 2023
@pavlidvg
Copy link
Author

If merged, I think this addresses pretty much everything we discussed except link readability. Let me know if you want any changes or corrections.

I also don't do any qss file validation, mostly because I don't know an easy way for it. Any invalid files imported are still added as an action, but the default theme is set when applying them. I tried several cutsom qss files online but did not include them because most of them had contrast issues on several widgets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants