ogarcia / opensudoku

Open Source Sudoku game for Android.
GNU General Public License v3.0
328 stars 144 forks source link

Full App UI Themes #52

Closed jlnordin closed 4 years ago

jlnordin commented 4 years ago

This pull request adds support for fully changing the app's color theme when selecting a theme from the theme selector. Prior to this change the theme selector was limited to changing the Sudoku board's colors, but was not able to change the overall colors of the app's UI.

New features:

New Themes

New Themes This PR includes 12 new themes, 6 light and 6 dark:

Technical Details

In order to support changing the app UI colors, almost all of the UI activities needed to be updated to no longer hard-code any colors. The ThemeUtils class has been updated with lots of supporting functions to make this easier. I've also explicitly updated the contrast on some of the UI controls, such as the note selection buttons in the popup input method dialog, to ensure the app theme is respected and it is clear what notes are selected.

Supporting customizing the app UI colors for a custom theme proved to be more difficult. All styles need to be pre-programmed into the app manifest as resources. So, in order to support the user choosing any theme colors he wants, we support 256 colors for the primary, secondary, and accent colors. When a color is selected in the custom theme UI, we quantize the selection to the closest match in the resources. These 256 colors are the material design colors that are part of Google's UI design guidelines. All of the built-in themes were also made by consulting this list of material design colors.

Finally, the feature to create a theme based on a single color has a set of heuristics I came up with regarding generating light and dark color variations, as well as making use of the Android ColorUtils class to calculate contrast ratios and pick accent colors based on a hue shift of the original color.

Opinionated Change

The one opinionated change in this PR which I'm willing to take back is that I've changed the theme that is selected on first launch to be the "Open Sudoku" theme instead of the theme called "Default". This means new installations of the app will start with the "Open Sudoku" theme. I'm willing to take this part out, but I felt that the "Open Sudoku" theme is a better default given it is designed around the colors of the Open Sudoku iconography.

Testing

jlnordin commented 4 years ago

By the way, this PR addresses issue #41 .

ogarcia commented 4 years ago

WOW!! You rocks!

ogarcia commented 4 years ago

Sorry for delay, too much work.

I think that is good idea that default theme are the new "Open Sudoku" created by you and I like these name.

What do you think that use "Open Sudoku" as default and rename current default as "Dark" and put it between "Ruby" and "Paper"?

ogarcia commented 4 years ago

What do you think of add an option in the theme editor to change colorButtonNormal?

jlnordin commented 4 years ago

I didn't want anyone to have their theme changed too dramatically when they upgrade to the new version, which is why I kept the name "default". I suppose I could keep the theme code as "default" for purposes of upgrading existing users and change the display name in the UI to "Dark" (and move it down next to Paper). How does that sound?

As for colorButtonNormal, I would need to add another 256 styles to properly support the custom theme editor. It's doable, but I found that being able to just change the accent and title bar colors give the UI enough personality without making things unreadable.

ogarcia commented 4 years ago

I didn't want anyone to have their theme changed too dramatically when they upgrade to the new version, which is why I kept the name "default". I suppose I could keep the theme code as "default" for purposes of upgrading existing users and change the display name in the UI to "Dark" (and move it down next to Paper). How does that sound?

Yes this is a good idea.

As for colorButtonNormal, I would need to add another 256 styles to properly support the custom theme editor. It's doable, but I found that being able to just change the accent and title bar colors give the UI enough personality without making things unreadable.

Now if you select bad colors (as same color for primary and accent) you have things unreadable. I think it is preferable that the user has more freedom even at the cost of being able to "break" the system. And in any case the user can always choose a predefined theme again and fix the broken. I say this because some colorblind users ask for this change in the past.

jlnordin commented 4 years ago

Can't argue with that! :-)

I'll work on these changes this week and update the PR.

On Tue, Jan 28, 2020, 3:23 AM Óscar García Amor notifications@github.com wrote:

I didn't want anyone to have their theme changed too dramatically when they upgrade to the new version, which is why I kept the name "default". I suppose I could keep the theme code as "default" for purposes of upgrading existing users and change the display name in the UI to "Dark" (and move it down next to Paper). How does that sound?

Yes this is a good idea.

As for colorButtonNormal, I would need to add another 256 styles to properly support the custom theme editor. It's doable, but I found that being able to just change the accent and title bar colors give the UI enough personality without making things unreadable.

Now if you select bad colors (as same color for primary and accent) you have things unreadable. I think it is preferable that the user has more freedom even at the cost of being able to "break" the system. And in any case the user can always choose a predefined theme again and fix the broken. I say this because some colorblind users ask for this change in the past.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/52?email_source=notifications&email_token=AOETRWACWGN4GZQT73C2THTRAAITRA5CNFSM4KL225A2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKC6JFI#issuecomment-579200149, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWFL4CN7OSBYUHQXNR3RAAITRANCNFSM4KL225AQ .

jlnordin commented 4 years ago

Ok, the new changes are up now.

On Tue, Jan 28, 2020, 9:32 AM Justin Nordin jlnordin@gmail.com wrote:

Can't argue with that! :-)

I'll work on these changes this week and update the PR.

On Tue, Jan 28, 2020, 3:23 AM Óscar García Amor notifications@github.com wrote:

I didn't want anyone to have their theme changed too dramatically when they upgrade to the new version, which is why I kept the name "default". I suppose I could keep the theme code as "default" for purposes of upgrading existing users and change the display name in the UI to "Dark" (and move it down next to Paper). How does that sound?

Yes this is a good idea.

As for colorButtonNormal, I would need to add another 256 styles to properly support the custom theme editor. It's doable, but I found that being able to just change the accent and title bar colors give the UI enough personality without making things unreadable.

Now if you select bad colors (as same color for primary and accent) you have things unreadable. I think it is preferable that the user has more freedom even at the cost of being able to "break" the system. And in any case the user can always choose a predefined theme again and fix the broken. I say this because some colorblind users ask for this change in the past.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/52?email_source=notifications&email_token=AOETRWACWGN4GZQT73C2THTRAAITRA5CNFSM4KL225A2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKC6JFI#issuecomment-579200149, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWFL4CN7OSBYUHQXNR3RAAITRANCNFSM4KL225AQ .

ogarcia commented 4 years ago

Perfect. Between today and tomorrow I'll merge your PRs and upload it as new version in play store.

jlnordin commented 4 years ago

I actually have 2 more follow on PRs I'd like to get in before uploading a new version, if possible.

After themes and note highlighting are merged, I have:

  1. PR to add undo and settings buttons to the action bar when playing. This is pretty small.

  2. PR to add a title screen with a "resume" feature to automatically launch to the most recently played game.

If you want to upload a new version ahead of those changes, I won't object though. :-)

Just wanted to give you a heads up on my immediate ideas.

On Thu, Jan 30, 2020, 3:59 AM Óscar García Amor notifications@github.com wrote:

Perfect. Between today and tomorrow I'll merge your PRs and upload it as new version in play store.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/52?email_source=notifications&email_token=AOETRWGUJZXUKYY3OXAWJHLRAK6IHA5CNFSM4KL225A2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKKXIGA#issuecomment-580219928, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWFJE3YUGZROIIBOT4DRAK6IHANCNFSM4KL225AQ .

ogarcia commented 4 years ago

Ok. I will wait to these PR and merge all together.

jlnordin commented 4 years ago

Thanks!

Those other changes are based on this app themes change, so I'll still need this PR to merge first.

On Fri, Jan 31, 2020, 1:04 AM Óscar García Amor notifications@github.com wrote:

Ok. I will wait to these PR and merge all together.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/52?email_source=notifications&email_token=AOETRWC4ZY7OW5OHUR66FRLRAPSSXA5CNFSM4KL225A2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKN7UDY#issuecomment-580647439, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWGO7VG3LM52SPDIOSDRAPSSXANCNFSM4KL225AQ .

ogarcia commented 4 years ago

Merged! One thing. I move the modified/added strings to bottom of file to make easy the changes to translators ;-)