Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration #4382

bhaktideshmukh · 2022-06-05T13:45:08Z

Explanation

Fixes part of #4195

Mock link :-- https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/249c3349-605b-4569-bbe9-1a017ab90101/specs/

This PR handles:--

Adds dark mode support to input interactions in QuestionPlayer and Exploration
Handles color of ic_arrow_drop_grey.xml and ic_arrow_right_grey.xml
Adds dark mode to submitted_answers
Changed the cursor color

Screenshots

Default	Dark mode

Default	Dark mode

Default	Dark mode

Default	Dark mode

Default	Dark mode

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

rt4914

@bhaktideshmukh Please add screenshots for Exploration player too.

rt4914 · 2022-06-06T20:25:43Z

app/src/main/res/drawable/edit_text_background_border_blue.xml

  <stroke
    android:width="2dp"
-    android:color="@color/color_def_oppia_dark_blue" />
+    android:color="@color/component_color_shared_input_interaction_edit_text_border_color" />


Why are you naming it like
input_interaction_edit_text
and above you are naming it as text_input_layout_

shouldn't these be consistent?

Why are you naming it like input_interaction_edit_text and above you are naming it as text_input_layout_

shouldn't these be consistent?

Correct these should be consistent but in styles.xml, for input interaction some are named as ...text_input... and some as ..input_interaction... so this was little confusion that which convention should be followed since for already existing color I have not created a new color and have used the existing one.

In that case lets fix this confusion in this PR atleast for this file.

In that case lets fix this confusion in this PR atleast for this file.

I think ...input_interaction... sounds better to me.
So should I replace ...text_input... with ...input_interaction...?

app/src/main/res/drawable/submitted_answer_background.xml

bhaktideshmukh · 2022-06-07T08:45:29Z

@bhaktideshmukh Please add screenshots for Exploration player too.

Done

rt4914

Make following changes:

The hint color should be changed https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/4744bfae-84be-44fa-a27d-9a252fdfc2f9/specs/
I don't have any reference for cursor-color but I believe we can use this one https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/d80a87ee-06df-4d82-ab8e-8311b9adccf6/specs/
When input-interaction is not. selected I think the border should have similar width as that of selected state. Reference: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/4744bfae-84be-44fa-a27d-9a252fdfc2f9/specs/

bhaktideshmukh · 2022-06-14T19:02:39Z

Make following changes:

1. The hint color should be changed https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/4744bfae-84be-44fa-a27d-9a252fdfc2f9/specs/

2. I don't have any reference for cursor-color but I believe we can use this one https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/d80a87ee-06df-4d82-ab8e-8311b9adccf6/specs/

3. When input-interaction is **not**. selected I think the border should have similar width as that of selected state. Reference: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/4744bfae-84be-44fa-a27d-9a252fdfc2f9/specs/

First is done
For the cursor color, for now I have changed the color in theme.xml since the solution you suggested of using android:textCursorDrawable wasn't working. But we can still revert it and can go with some other option.
Third is done

Have updated the screenshots according to the new changes.

rt4914

@bhaktideshmukh PTAL Thanks. Somewhere on lines of this solution https://stackoverflow.com/a/7238526

rt4914 · 2022-06-18T08:23:52Z

app/src/main/res/values-v21/themes.xml

@@ -12,7 +12,7 @@
  <style name="OppiaThemeWithoutActionBar" parent="Theme.MaterialComponents.Light.NoActionBar.Bridge">
    <item name="colorPrimary">@color/color_palette_primary_color</item>
    <item name="colorPrimaryDark">@color/color_palette_primary_dark_color</item>
-    <item name="colorAccent">@color/color_palette_accent_color</item>
+    <item name="colorAccent">@color/component_color_shared_edit_text_cursor_color</item>


This should not be changed here because colorAccent must be getting used at various other places too. and it will affect other UI elements when changed.

Changing it in theme is fine but we should be changing the exact curor_color to some similar attribute and not its parent attribute like colorAccent.

Also the color in lightmode has changed because of latest commit. It should not be black, it should be blue https://xd.adobe.com/view/3dca36c2-5115-419c-b25e-0f10526b077c-6899/screen/5ef661fa-8995-45b2-b10d-64c1b5cc7890/

oppiabot · 2022-06-25T08:53:07Z

Hi @bhaktideshmukh, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

bhaktideshmukh · 2022-06-25T13:14:00Z

@rt4914, I have made the changes the way you suggested but upon clicking input interaction, app is crashing.
I am not getting what's the cause behind this PTAL whats wrong.
Thanks.

BenHenning · 2022-06-27T05:05:06Z

@rt4914, I have made the changes the way you suggested but upon clicking input interaction, app is crashing. I am not getting what's the cause behind this PTAL whats wrong. Thanks.

Hi @bhaktideshmukh. This sounds like it might be a good case for a debug doc (see https://github.com/oppia/oppia/wiki/Debugging-Docs) as that provides more context as to what you tried, what you're specifically observing, and what you think might be going wrong but are unsure of how to proceed.

Note that it doesn't actually need to be a Google doc--you can add the same information as a comment to this thread to provide context for reviewers. Note that without this information, it can be quite difficult to help since reviewers generally do not know as much about what you've tried as you do.

…ndroid into input_interaction

bhaktideshmukh · 2022-07-06T10:56:53Z

@rt4914, I have made the changes the way you suggested but upon clicking input interaction, app is crashing. I am not getting what's the cause behind this PTAL whats wrong. Thanks.

Hi @bhaktideshmukh. This sounds like it might be a good case for a debug doc (see https://github.com/oppia/oppia/wiki/Debugging-Docs) as that provides more context as to what you tried, what you're specifically observing, and what you think might be going wrong but are unsure of how to proceed.

Note that it doesn't actually need to be a Google doc--you can add the same information as a comment to this thread to provide context for reviewers. Note that without this information, it can be quite difficult to help since reviewers generally do not know as much about what you've tried as you do.

I guess there was some glitch from my side, I have updated the files and app is not crashing now and is working properly so I don't think there is any need for the debugging doc.

app/src/main/res/drawable/color_cursor.xml

rt4914 · 2022-07-11T18:33:15Z

app/src/main/res/drawable/edit_text_background_border_blue.xml

  <stroke
    android:width="2dp"
-    android:color="@color/color_def_oppia_dark_blue" />
+    android:color="@color/component_color_shared_input_interaction_edit_text_border_color" />


In that case lets fix this confusion in this PR atleast for this file.

rt4914

@bhaktideshmukh PTAL at all open comments.

app/src/main/res/values-night/color_palette.xml

bhaktideshmukh · 2022-07-12T11:53:52Z

@bhaktideshmukh PTAL at all open comments.

Done.

rt4914

LGTM, thanks.

…er and Exploration (#4382)" This reverts commit c3b42a4.

…er and Exploration (#4382)" (#4478) This reverts commit c3b42a4.

added dark mode for input interaction

eada89b

bhaktideshmukh requested a review from rt4914 as a code owner June 5, 2022 13:45

bhaktideshmukh assigned rt4914 Jun 5, 2022

rt4914 suggested changes Jun 6, 2022

View reviewed changes

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jun 6, 2022

bhaktideshmukh and others added 3 commits June 7, 2022 13:05

updated naming convention

3a7bf55

added error_color

f0d11d1

Merge branch 'develop' into input_interaction

d19a2ef

bhaktideshmukh assigned rt4914 and unassigned bhaktideshmukh Jun 7, 2022

rt4914 suggested changes Jun 13, 2022

View reviewed changes

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jun 13, 2022

bhaktideshmukh and others added 3 commits June 14, 2022 18:30

added hint color

253833f

Merge branch 'develop' into input_interaction

56d1a6e

changes cursor color

0473c4f

bhaktideshmukh assigned rt4914 and unassigned bhaktideshmukh Jun 14, 2022

rt4914 suggested changes Jun 18, 2022

View reviewed changes

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jun 18, 2022

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Jun 25, 2022

changed cursor color

bb762fe

oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Jun 25, 2022

bhaktideshmukh assigned rt4914 Jun 25, 2022

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jun 30, 2022

bhaktideshmukh added 2 commits July 6, 2022 15:48

updated cursor_color

bb49826

Merge branch 'input_interaction' of github.com:bhaktideshmukh/oppia-a…

e8cb83c

…ndroid into input_interaction

bhaktideshmukh assigned rt4914 and unassigned bhaktideshmukh Jul 6, 2022

resolving failing checks

df2ba09

rt4914 reviewed Jul 11, 2022

View reviewed changes

rt4914 suggested changes Jul 11, 2022

View reviewed changes

app/src/main/res/values-night/color_palette.xml Show resolved Hide resolved

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jul 11, 2022

bhaktideshmukh and others added 2 commits July 12, 2022 16:02

Merge branch 'develop' into input_interaction

59f0be3

resolving failing checks

a277bc3

bhaktideshmukh assigned rt4914 and unassigned bhaktideshmukh Jul 12, 2022

rt4914 assigned bhaktideshmukh and unassigned rt4914 Jul 16, 2022

bhaktideshmukh and others added 2 commits July 17, 2022 11:46

added new hint color

1a0c1d8

Merge branch 'develop' into input_interaction

d0f2d70

bhaktideshmukh assigned rt4914 and unassigned bhaktideshmukh Jul 17, 2022

rt4914 approved these changes Jul 19, 2022

View reviewed changes

rt4914 merged commit c3b42a4 into oppia:develop Jul 19, 2022

BenHenning added a commit that referenced this pull request Jul 29, 2022

Revert "Fixes part of #4195 : Added dark mode support to QuestionPlay…

ca27725

…er and Exploration (#4382)" This reverts commit c3b42a4.

BenHenning mentioned this pull request Jul 29, 2022

Fix #4477: Revert "Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration" #4478

Merged

rt4914 pushed a commit that referenced this pull request Jul 29, 2022

Revert "Fixes part of #4195 : Added dark mode support to QuestionPlay…

ec9a891

…er and Exploration (#4382)" (#4478) This reverts commit c3b42a4.

bhaktideshmukh mentioned this pull request Aug 29, 2022

Fixes part of #4195:Added dark mode support to QuestionPlayer and ExplorationPlayer #4540

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration #4382

Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration #4382

bhaktideshmukh commented Jun 5, 2022 •

edited

rt4914 left a comment

rt4914 Jun 6, 2022

bhaktideshmukh Jun 7, 2022

rt4914 Jul 11, 2022

bhaktideshmukh Jul 12, 2022

bhaktideshmukh commented Jun 7, 2022

rt4914 left a comment

bhaktideshmukh commented Jun 14, 2022 •

edited

rt4914 left a comment

rt4914 Jun 18, 2022

oppiabot bot commented Jun 25, 2022

bhaktideshmukh commented Jun 25, 2022

BenHenning commented Jun 27, 2022

bhaktideshmukh commented Jul 6, 2022

rt4914 Jul 11, 2022

rt4914 left a comment

bhaktideshmukh commented Jul 12, 2022

rt4914 left a comment

Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration #4382

Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration #4382

Conversation

bhaktideshmukh commented Jun 5, 2022 • edited

Explanation

Essential Checklist

For UI-specific PRs only

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Jun 6, 2022

Choose a reason for hiding this comment

bhaktideshmukh Jun 7, 2022

Choose a reason for hiding this comment

rt4914 Jul 11, 2022

Choose a reason for hiding this comment

bhaktideshmukh Jul 12, 2022

Choose a reason for hiding this comment

bhaktideshmukh commented Jun 7, 2022

rt4914 left a comment

Choose a reason for hiding this comment

bhaktideshmukh commented Jun 14, 2022 • edited

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Jun 18, 2022

Choose a reason for hiding this comment

oppiabot bot commented Jun 25, 2022

bhaktideshmukh commented Jun 25, 2022

BenHenning commented Jun 27, 2022

bhaktideshmukh commented Jul 6, 2022

rt4914 Jul 11, 2022

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

bhaktideshmukh commented Jul 12, 2022

rt4914 left a comment

Choose a reason for hiding this comment

bhaktideshmukh commented Jun 5, 2022 •

edited

bhaktideshmukh commented Jun 14, 2022 •

edited