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] Fix error message being unreadable in night mode #7815

Merged
merged 1 commit into from
Apr 20, 2024

Conversation

SRSAS
Copy link
Contributor

@SRSAS SRSAS commented Apr 4, 2024

closes #6481

When the user has their device in light mode, but the app is in night mode, the error popups on EditText dialogs have the same background and font color, therefore being unreadable.

EditText error popups cannot have styles directly applied to them, thus the less disruptive fix was to call the setError method on the TextInputLayout wrapper object instead of on the EditText object.

Drawable resources were added to maintain the error icon that appears when an error popup is present.

This fix changes the error popup appearence. However, it can now more easily be customized by applying a style to the TextInputLayout element.

New error popup:

screenshot

@SRSAS SRSAS requested a review from a team as a code owner April 4, 2024 23:55
@Jean-BaptisteC
Copy link
Member

Jean-BaptisteC commented Apr 5, 2024

Thanks for your contribution
IMO, it's not the right correction. With this change we return to the past -> check PR #4569
The right solution is to add a new style applied on field -> https://stackoverflow.com/questions/33709066/how-to-set-textinputlayout-error-message-colour

@SRSAS
Copy link
Contributor Author

SRSAS commented Apr 9, 2024

I continued analyzing the problem, and found out that there was an issue in the inherited styles when the app is in dark mode but the device is in light mode. This is because the MwmTheme.Night.Base style inherits from Theme.MaterialComponents.DayNight.NoActionBar.Bridge which only changes to night mode if the device is in night mode, or if the application is explicitly set in night mode.

To fix I removed the changes from the original commit, and used the UiModeManager object in the ThemeSwitcher, to change the application to the correct mode, in a new commit.

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 does it work on older devices?

@SRSAS
Copy link
Contributor Author

SRSAS commented Apr 9, 2024

I tested on Pixel 2 - Android 5 and the bug persists with API 30 or below.
I found a fix for this. Should I redo the commit?

@Jean-BaptisteC
Copy link
Member

Amend your commit and add the new fix

When the user has their device in light mode, but the app is in night
mode, some attributes in the
Theme.MaterialComponents.DayNight.NoActionBar.Bridge style are still
inheriting from the light theme. This happens because the application's
ui mode is not being properly switched to night mode.

To fix this, added the UiModeManager in the ThemeSwitcher, in order to
properly change the ui mode.

closes #6481

Signed-off-by: Sebastiao Sousa <sebastiao.sousa@tecnico.ulisboa.pt>
@SRSAS
Copy link
Contributor Author

SRSAS commented Apr 9, 2024

Done!

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.

Thank you and respect for digging deeper! Will it properly work when the #749 is finished? E.g. when will the "As in the system" dark or light mode be used for the app theme?

P.S. Any help in finishing #749 is appreciated, it was already done for iOS...

@SRSAS
Copy link
Contributor Author

SRSAS commented Apr 10, 2024

It should work! But it must be handled correctly in the new implementation.

The android documentation recommends using UiModeManager.setApplicationNightMode() for API 31 or higher. However, I've been exploring how to set it so the app theme follows the device ui mode, but the only way I found to do it is by always using AppCompatDelegate.setDefaultNightMode() instead of the UiModeManager.

So, the way I see it, when the android implementation is done, the UiModeManager should be replaced with AppCompatDelegate, and when the app should follow the system UI mode, the MODE_NIGHT_FOLLOW_SYSTEM constant should be used.

@@ -39,6 +39,7 @@ Code contributions:
Caspar Nuël <casparnuel@yandex.com>
Konstantin Pastbin
Nishant Bhandari <nishantbhandari0019@gmail.com>
Sebastiao Sousa <sebastiao.sousa@tecnico.ulisboa.pt>
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I forgot about existence of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove this from the commit?

@SRSAS
Copy link
Contributor Author

SRSAS commented Apr 20, 2024

Hey! Anything else I should change?

@biodranik biodranik merged commit fd2a1ee into organicmaps:master Apr 20, 2024
6 checks passed
@biodranik
Copy link
Member

It's good to go, thanks, and welcome to the club!

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.

Wrong text color on error message
4 participants