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

Improved behavior of the "Edit day or night colors?" dialog #107

Closed
wants to merge 1 commit into from

Conversation

BlackyHawky
Copy link
Contributor

Currently, Edit day or night colors? dialog appears when User-defined theme is selected in Theme variant and Theme variant (night) menus. (This is quite normal)

However, this window also appears when User-defined theme is defined in Theme variant menu but not selected in Theme variant (night) menu.

This PR therefore allows this dialog to appear only when User-defined theme is selected in both Theme variant menus.

To notice this "bad" behavior, you must have the "Auto day/night mode" toggle on.

@Helium314
Copy link
Owner

Not sure about this. While the question might not be necessary, it avoids users being confused becuase the user-defined night theme is not the same as the normal one. There is no other indicator, and I didn't want to add yet another usually disabled pref. It would be much better if it was simply possible to hide and show prefs instead of only removing them.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Aug 28, 2023

As you wish; I trust you to do what's best for Openboard and its users.

To be honest, I never use night or auto mode.

Yesterday I took the time to play with the application settings and was surprised to see that I was offered to edit the day or night theme when I hadn't chosen User-defined as the night theme.

I didn't think you'd done that on purpose.

In short, I had made this PR thinking of other users who might have been surprised like me.

So I'll close this PR; I don't mind at all.

@BlackyHawky BlackyHawky deleted the Appearance branch August 28, 2023 20:48
@Helium314
Copy link
Owner

I'm also not sure what is best...
Ideally a setting for user-defined night colors will show up on selecting the theme. Maybe I can implement this when re-doing the appearance settings.

@BlackyHawky
Copy link
Contributor Author

BlackyHawky commented Aug 29, 2023

Here's another approach in this commit ede2985 to avoid user confusion:

  1. Edit day or night colors? dialog appears only when User-defined theme is selected in both Theme variant menus

  2. If User-defined is only selected in Theme variant menu, when you click on Adjust theme colors the title of the dialog becomes Day mode color to adjust.
    Same behavior if you click on the Day button in window Edit day or night colors?.

  3. If User-defined is only selected in Theme variant (night mode) menu, when you click on Adjust theme colors the title of the dialog becomes Night mode color to adjust
    Same behavior if you click on the Night button in window Edit day or night colors?.

What do you think about it ?

@Helium314
Copy link
Owner

This is a good approach, though it might cause confusion in some other place:
If day/night more is off, it's not entirely clear that default is day more. But I think the user should understand it when the day/night preference is in view.
My concern here is that there is no day/night pref on older Android versions, so it might be confusing (but only a little I guess).

You removed the string select_color_to_adjust only from the default strings.xml, but not from other languages.
This is of course easily fixable, but it also means that situation for non-english languages is worse than before, until someone comes and translates the new strings to the 5 other languages of the old one.
Currently this is not too bad, but the more translations exist, the less I think a string should be exchanged for a similar one.

Do you think the UI would work with the old select_color_to_adjust string instead of day_color_to_adjust?

@BlackyHawky
Copy link
Contributor Author

This is a good approach, though it might cause confusion in some other place: If day/night more is off, it's not entirely clear that default is day more. But I think the user should understand it when the day/night preference is in view. My concern here is that there is no day/night pref on older Android versions, so it might be confusing (but only a little I guess).

In this case, we can let the title select_color_to_adjust only if User-defined is chosen in Theme variant and Auto day/night mode is toggle off.

I think, and this is only a personal opinion, that if a user activates the Auto day/night mode setting, he will necessarily want to modify either the personal day theme or the personal night theme (or both obviously).

However, if the Auto day/night mode option is disabled, having the title select_color_to_adjust is more than enough.

New commit 911c348 created to show you this.

You removed the string select_color_to_adjust only from the default strings.xml, but not from other languages.

Don't worry; my commit is just a test to show you the changes made so I haven't changed anything other than what is strictly necessary.

Do you think the UI would work with the old select_color_to_adjust string instead of day_color_to_adjust?

I'm sorry but I don't understand the meaning of your question.

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.

2 participants