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

[android] Implement Light, Dark, and System modes on android(Auto) #8309

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SRSAS
Copy link
Contributor

@SRSAS SRSAS commented May 27, 2024

closes #749

Changed from the existing night modes (on, off, auto), to "appearence" modes:
- Light;
- Dark;
- System;

System mode follows whatever mode the user's device is in. This implementation works for both normal android and AndroidAuto.

Main changes:
- added necessary strings to XML files;
- changed the position and option names of the "map style preferences";
- implemented system mode, and some helper methods on ThemeSwitcher.java and ThemeUtils.java;
- added uiMode changes detection to all activities;
- removed "auto" mode implementation;

Co-authored-by: Francisco Nael Salgado [email protected]

@Jean-BaptisteC
Copy link
Member

Really good changes 🙂
IMO we want to keep auto mode to continue to switch in dark mode when the night is coming

@SRSAS
Copy link
Contributor Author

SRSAS commented May 27, 2024

Thanks!
I can add it in. But should it be kept as a 4th option, a toggle, or automatically activated when the user enters navigation mode?

@Jean-BaptisteC
Copy link
Member

Jean-BaptisteC commented May 27, 2024

I prefer 4 options
Others things:

  • Update translations in string.txt and regenerate translations
  • I have started to test, switch between dark and light theme require to go back to main screen to refresh theme.

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

Thanks! It would be better to merge this PR first, and implement "Follow the sun" behavior separately, if needed.

android:windowSoftInputMode="stateVisible"/>
<activity
android:name="app.organicmaps.widget.placepage.PlaceDescriptionActivity"
android:configChanges="uiMode"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to automatically add it to any activity without explicitly specifying everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The XML attributes are only inherited by activities that actualy inherit (in java) other activities. So we were able to reduce the number of 'android:configChanges="uiMode"' by adding the BaseMwmFragmentActivity to the manifest and giving it that attribute.
However, any activity that specifies its configChanges, has to have "uiMode" set explicitly

@@ -103,6 +105,11 @@ protected void onDestroy()
mPermissionRequest = null;
}

@Override
public void onConfigurationChanged(@NonNull Configuration newConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public void onConfigurationChanged(@NonNull Configuration newConfig) {
public void onConfigurationChanged(@NonNull Configuration newConfig)
{

@@ -128,6 +130,12 @@ public void onPostResume()
}
}

@Override
public void onConfigurationChanged(@NonNull Configuration newConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public void onConfigurationChanged(@NonNull Configuration newConfig) {
public void onConfigurationChanged(@NonNull Configuration newConfig)
{

Comment on lines 96 to 98
if (themeMode.equals(followSystemTheme)) {
return ThemeMode.FOLLOW_SYSTEM;
}
Copy link
Member

Choose a reason for hiding this comment

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

One-liners don't need braces.

Suggested change
if (themeMode.equals(followSystemTheme)) {
return ThemeMode.FOLLOW_SYSTEM;
}
if (themeMode.equals(followSystemTheme))
return ThemeMode.FOLLOW_SYSTEM;

mAutoThemeChecker.run();
return;
}
System.out.println("theme:" + theme);
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

We really need to add a formatter. A lot of time and effort is spent on these nit comments.

@Framework.MapStyle
int oldStyle = Framework.nativeGetMapStyle();

private int getStyle(@NonNull String theme) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private int getStyle(@NonNull String theme) {
private int getStyle(@NonNull String theme)
{

else
AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_YES);

if (ThemeUtils.isNightTheme(mContext, theme)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (ThemeUtils.isNightTheme(mContext, theme)) {
if (ThemeUtils.isNightTheme(mContext, theme))
{

else
AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO);

} else {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else {
}
else
{

@@ -45,6 +47,25 @@ public static int getResource(@NonNull Context context, @AttrRes int style, @Att
return VALUE_BUFFER.resourceId;
}

public static String getAndroidTheme(@NonNull Context context) {
Copy link
Member

Choose a reason for hiding this comment

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

Please fix braces in other places.

@@ -97,39 +97,39 @@
android:summary="@string/enable_show_on_lock_screen_description"
android:defaultValue="true"
android:order="17"/>
<ListPreference
android:key="@string/pref_map_style"
Copy link
Member

Choose a reason for hiding this comment

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

Wrong indentation?

SRSAS and others added 2 commits May 28, 2024 01:21
closes organicmaps#749

Changed from the existing night modes (on, off, auto), to "appearence"
modes:
	- Light;
	- Dark;
	- System;

System mode follows whatever mode the user's device is in. This
implementation works for both normal android and AndroidAuto.

Main changes:
	- added necessary strings to XML files;
	- changed the position and option names of the "map style
	  preferences";
	- implemented system mode, and some helper methods on
	  ThemeSwitcher.java and ThemeUtils.java;
	- added uiMode changes detection to all activities;
	- removed "auto" mode implementation;

Signed-off-by: Sebastiao Sousa <[email protected]>
Co-authored-by: Francisco Nael Salgado <[email protected]>
Added system, light, and dark to strings.txt.
Modified pref_map_style_title from Night Mode to Appearance

Signed-off-by: Sebastiao Sousa <[email protected]>
Co-authored-by: Francisco Nael Salgado <[email protected]>
@SRSAS
Copy link
Contributor Author

SRSAS commented May 28, 2024

Sorry for the formatting mistakes!
We think we've corrected the bug. We also have added the strings to strings.txt and regenerated.

Signed-off-by: Sebastião Andrade e Sousa <[email protected]>
Co-authored-by: Francisco Nael Salgado <[email protected]>
eu = Argi
fa = نور
fi = Vaalea
fr = Claire
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fr = Claire
fr = Clair

@biodranik biodranik added this to the Needs alpha/beta testing milestone May 28, 2024
@rtsisyk rtsisyk self-assigned this Jun 9, 2024
@rtsisyk
Copy link
Contributor

rtsisyk commented Jun 9, 2024

Hi, thanks for contributing. This PR is my backlog and I will try to review and test it quickly. Please be patient.

@biodranik
Copy link
Member

@SRSAS can you please rebase your PR? It's easy to avoid conflicts by removing the strings regeneration commit, and then by regenerating strings again.

@rtsisyk can you please test this PR? Many users asked for it.

Copy link
Member

@Jean-BaptisteC Jean-BaptisteC left a comment

Choose a reason for hiding this comment

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

❌ Pixel 6 - Android 14
How system value works on device using Android 8 or Android 5 (In this version native settings to update theme device doesn't exist) ?
Change Android theme settings is not seen by the app when users is in OSM profile fragment and about fragment

@biodranik
Copy link
Member

Change Android theme settings is not seen by the app when users is in OSM profile fragment and about fragment

Are these fragments recolored when they are closed/opened again?

@Jean-BaptisteC
Copy link
Member

To refresh theme, that's require switch to another screen on app.
Maybe it's missing property in manifest for thes activities?

@SRSAS
Copy link
Contributor Author

SRSAS commented Jun 29, 2024

@SRSAS can you please rebase your PR? It's easy to avoid conflicts by removing the strings regeneration commit, and then by regenerating strings again.

@rtsisyk can you please test this PR? Many users asked for it.

Will do

@SRSAS
Copy link
Contributor Author

SRSAS commented Jun 29, 2024

❌ Pixel 6 - Android 14 How system value works on device using Android 8 or Android 5 (In this version native settings to update theme device doesn't exist) ? Change Android theme settings is not seen by the app when users is in OSM profile fragment and about fragment

I will look into it!

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.

Synchronize night mode to phone system settings (Android + Android Auto)
4 participants