Fix part of #4030 : Implementing dark mode to various screens.

[ayush0402]

Explanation

Fix part of #4030
Link to the original PR having discussions over the decided approach : link

Summary of the approach followed in this PR :

• Introduced new colors in color_defs.xml following decided snake_case naming convention. colors_def.xml should strictly contain actual color name irrespective of their intended use with their hex color code declaration.

<color name="lime_green">#90EE90</color>

• Then color_palette.xml (day/night) splits the colors (to be used by component_colors.xml) according to chosen theme. color_palette.xml should not contain any name tied to the intended component to be used on, rather should have generic naming of the colors.

Don't :

 <color name="add_profile_background_color">@color/oppia_background_black</color>
 <color name="text_input_layout_error_color">@color/error_red</color>

Do :

 <color name="background_color">@color/oppia_background_black</color>
 <color name="error_color">@color/error_red</color>

• component_colors.xml then only references color_palette.xml. It uses UI component specific names. Component colors should be shared very little outside of their respective views/fragments/activities.

  <color name="shared_text_input_edit_text_cursor_color">@color/primary_text_color</color>
  <color name="shared_activity_toolbar_color">@color/toolbar_color</color>
  <!--Styles.xml-->
  <color name="shared_text_input_layout_text_color">@color/primary_text_color</color>
  <color name="shared_input_interaction_edit_text_text_color">@color/primary_text_color</color>
  <color name="shared_text_input_layout_background_color">@color/text_input_background_color</color>
  <!--Admin Auth Activity-->
  <color name="admin_auth_secondary_text_color">@color/description_text_color</color>
  <color name="admin_auth_layout_background_color">@color/background_color</color>
  <!--Add Profile Activity-->
  <color name="add_profile_activity_label_text_color">@color/primary_text_color</color>
  <color name="add_profile_activity_switch_text_color">@color/dark_text_color</color>

• colors_migrating.xml contains color declarations which are supposed to be in color_defs.xml but has not been renamed yet to have actual color name instead of names linked to their use and components. This is a temporary measure to make sure other 4 color files follows the convention decided for them.

Note: colors declared in component_colors.xml and color_palette.xml should not use "oppia_" prefix to avoid redundancy and must have _color suffix. And just opposite applies to color_defs.xml

Link to comment by Ben on this PR summarising the approach.

Screenshots :

Light	Dark

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

• Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
• For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
• Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
• Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

[ayush0402]

@BenHenning I will be implementing dark mode to some layouts in this PR so I just wanted to confirm the approach by which I did it for one layout (Add profile activity) if this is good then I will be pushing more layouts similarly.

Light	Dark

Also I had two problems with this...

• I am using a different primary color as per the provided mocks but by default the Action bar is set to primary color only. I tried using this for changing the color of Action bar like...
  (Color of action bar is same in both light and dark themes).

  <!--ACTION BAR STYLE-->
  <style name="OppiaActionBar" parent="Widget.AppCompat.Light.ActionBar">
    <item name="android:background">@color/oppia_action_bar_color</item>
  </style>

  <style name="OppiaTheme" parent="Theme.MaterialComponents.Light.DarkActionBar.Bridge">
    <item name="actionBarStyle">@style/OppiaActionBar</item>
  </style>

but this is not working.

• Another issue was about TextInputLayout/TextInputEditText, I am unable to find which attribute for the component is responsible for color of the field name when it is not selected as shown in this video.

vokoscreen-2021-12-13_00-00-26.mp4

Please let me know if you have any suggestions for these.

[BenHenning]

Apologies @ayush0402, this will probably take me a bit to review so I won't be able to look at it until tomorrow at the earliest.

[BenHenning]

@ayush0402 regarding your issues:

1. I know that certain system components (such as the status bar) use different attributes across different SDK versions. You might need to continue searching to figure out which colors correspond to the action bar, and might even need to look at the source code for action bar itself. This isn't always straightforward, and this might actually be one of the trickier cases to solve in the project.

2. Same for text. You should look into the source code for the material component that we're using to see which attributes it uses for color selection for different parts of the layout.

I will be following up with high-level thoughts on the code soon, but I wanted to follow up on these points, first.

[BenHenning]

Thanks @ayush0402! And, apologies for the delay. I left some high-level thoughts in comments.

Overall, I think this is on the right approach but there may still be some confusion around the differences between the different color files. I also suspect you might benefit from adding a few more examples (especially layouts since this results in more component color cases which I expect is the more confusing one to figure out). Beyond that, it might be good to wait to finish the PR in its full until Rajat has a chance to review this (though that will mean waiting 1+ weeks for review). It's up to you if you want to make more progress in the meantime, just know that a lot of it might have to be redone if he has more major feedback later.

[BenHenning]

Hi. As of today, some main reviewers have taken time off for the next few weeks, so it may take a little while before we can look at this PR. We appreciate your patience while some of our team members recharge. We'll be fully returning on 4 January 2021.

[ayush0402]

Thanks, I figured out the issue with changing colors for the "ActionBar". Actually we were not using Action Bar at all for that screen, It was using a NoActionBar variant of the theme and it was actually a custom toolbar defined under layouts/toolbar.xml which was being for various layouts. Changing its properties under layouts/toolbar.xml works for changing the color other than colorPrimary now.

[ayush0402]

@BenHenning PTAL.

[BenHenning]

Thanks @ayush0402! I didn't look too closely at the theme files since I'm not entirely sure how we're going to be managing those long-term, but the rest of the PR generally looks good (just had a few comments--PTAL).

@rt4914 how should these new colors relate to themes? Should we maybe be restricting themes to only reference color_palette colors similar to color_components?

[ayush0402]

@BenHenning PTAL.

[rt4914]

Thanks @ayush0402! I didn't look too closely at the theme files since I'm not entirely sure how we're going to be managing those long-term, but the rest of the PR generally looks good (just had a few comments--PTAL).

@rt4914 how should these new colors relate to themes? Should we maybe be restricting themes to only reference color_palette colors similar to color_components?

@BenHenning yes agreed. Themes should connect to color_palette.xml only.

[BenHenning]

Thanks @ayush0402! I think the latest color files look really good, and I'm eager to see the rest of the colors get migrated over throughout the rest of the project. Thanks for your patience as we figured this out--I think it's moving toward much cleaner organization.

[BenHenning]

Though to check one thing: @ayush0402 and @rt4914, is this actually finishing #4030 or is there more color migration work to do prior to dark mode being considered done?

[ayush0402]

@BenHenning No, we still have many views/layouts to check and cover before dark mode can be considered complete. I earlier had thought of filing this issue for setting the base for the implementation and then create more issues for other layouts and maybe making them available to community for some help. But i guess I will create more PRs under the same issue and have this PR title renamed to Fix part of. I have updated the issue description to contain proper explanation for the approach with links to important comments.
@rt4914 What are your views about getting the help of other contributors by filing issues after I make couple more PRs under this issue only for them to have proper references?

[rt4914]

@BenHenning No, we still have many views/layouts to check and cover before dark mode can be considered complete. I earlier had thought of filing this issue for setting the base for the implementation and then create more issues for other layouts and maybe making them available to community for some help. But i guess I will create more PRs under the same issue and have this PR title renamed to Fix part of. I have updated the issue description to contain proper explanation for the approach with links to important comments. @rt4914 What are your views about getting the help of other contributors by filing issues after I make couple more PRs under this issue only for them to have proper references?

@ayush0402 Yes we can do that for sure. But I think we should create one more PR first and then do that. So that the new PR can act as sample PR and also we can be sure that everything is fully correct.