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

More Themes (Dark + Light) #750

Merged
merged 16 commits into from
Jun 19, 2023
Merged

Conversation

bynatejones
Copy link
Contributor

@bynatejones bynatejones commented Jun 19, 2023

Created a "Grey" option for Theme Color. Modified colors for both Dark and Light Theme.



Created a "Purple" option for Theme Color. Modified colors for both Dark and Light Theme.



Created a "Woodland" option for Theme Color. Modified colors for both Dark and Light Theme.



Created a "Beach" option for Theme Color. Modified colors for both Dark and Light Theme.



Created a "Crimson" option for Theme Color. Modified colors for both Dark and Light Theme.



The one thing that probably still needs a change is the fact that the Dialogs and the NavBar both use the .surface color, so in order to get the NavBar to be the same color as the background, the Dialogs must also be this color and can't be given a slight accent to separate it from the background. Example:


Two solutions I can think of:

  1. Separate the NavBar and Dialogs into different color values.
  2. Add a dark/light translucent background behind the dialogs to separate them from the background no matter what color they are. See Boost for this same behavior.

I think it might also be smart to separate the .primary, .secondary, and .tertiary color as their own user settings, so that themes don't have to be made for each possible primary color that a user might want to use. For instance, for the Grey (Dark) theme, I've made the .primary color just ever so slightly Blue to separate it more from the other UI on the home feed (since most of the other text is already grey). However, it would be nice if a user could change that to be green, pink, purple, etc etc (custom color picker, not hard coded colors) while still being able to use my general color scheme.

I can try my hand at any of the above solutions, just let me know what direction if any you'd like to go with it.


Also, question. I wasn't able to see where the URL color in comments is found in the code, since I'd like to be able to theme that too (and ultimately should probably have its own user setting override as well). Right now it seems to be hardcoded as this green color:


Let me know if you'd like me to make any changes. I plan on making more themes.

@bynatejones bynatejones changed the title Grey Theme (Dark + Light) More Themes (Dark + Light) Jun 19, 2023
@dessalines
Copy link
Member

The one thing that probably still needs a change is the fact that the Dialogs and the NavBar both use the .surface color, so in order to get the NavBar to be the same color as the background, the Dialogs must also be this color and can't be given a slight accent to separate it from the background. Example:

In that case, we should just find the correct Material3 theme variable that works best, for both. I'd rather not add custom theme hacks, or additional settings on top of M3, but just use the existing variables, to look good with all themes.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these, they all look really good.

@@ -832,9 +832,14 @@ enum class ThemeMode(val mode: Int) {

enum class ThemeColor {
Dynamic,
Beach,
Blue,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a merge from main, and you'll see that this enum has i18n strings now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add i18n strings to this, just liken ThemeMode has above it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've modified them to use i18n strings and added the English translations in the strings.xml file

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@dessalines dessalines merged commit 288f589 into LemmyNet:main Jun 19, 2023
1 check passed
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.

None yet

2 participants