Fix #5086 : Implement Dark Mode support for NPS Survey #5104

MohitGupta121 · 2023-07-25T06:44:42Z

Explanation

Fix #5086 : Implement full Dark Mode support for NPS Survey

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

NPS Survey Welcome Screen

Light Mode	Dark Mode

NPS Survey Question screen (Button Disabled)

Light Mode	Dark Mode

NPS Survey Question screen (Button Enabled)

Light Mode	Dark Mode

NPS Survey Score Screen

Light Mode	Dark Mode

NPS Survey Final comment screen (Button Enabled)

Light Mode	Dark Mode

NPS Survey Final comment screen (Button Disabled)

Light Mode	Dark Mode

NPS Survey Thanks You screen

Light Mode	Dark Mode

NPS Survey Exit Dialog

Light Mode	Dark Mode

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

MohitGupta121

@adhiamboperes PTAL, Thanks!

adhiamboperes

Thanks a lot @MohitGupta121! I just had two minor comments:

Can I see how the 'NPS Survey Final comment screen' looks without text and the submit button disabled?
Is there a way to make the progress bar look like the one in light mode (I don't mind too much, but let's give it a try).

Otherwise this looks really great!

oppiabot · 2023-07-25T10:11:59Z

Unassigning @adhiamboperes since the review is done.

oppiabot · 2023-07-25T10:12:02Z

Hi @MohitGupta121, it looks like some changes were requested on this pull request by @adhiamboperes. PTAL. Thanks!

MohitGupta121 · 2023-07-25T14:18:34Z

@adhiamboperes PTAL Thanks, I updated the PR description with button disabled final comment screen.

Regarding progress bar I don't think there is need to change the theme/ color of that because we using the same progress bar in Exploration Fragments also. So I think that already suggested by designer in past. If we change then it's look in-consistence in both places. Actually now in question fragment we not showing progress bar so here is the figma design by designer for Dark-Mode and Light-Mode

Also I think we need to introduce NPS Survey layout for landscape version because you can see the output, screen is not scrollable like welcome screen is incomplete and cutting from bottom:

adhiamboperes · 2023-07-25T19:13:21Z

@adhiamboperes PTAL Thanks, I updated the PR description with button disabled final comment screen.

Regarding progress bar I don't think there is need to change the theme/ color of that because we using the same progress bar in Exploration Fragments also. So I think that already suggested by designer in past. If we change then it's look in-consistence in both places. Actually now in question fragment we not showing progress bar so here is the figma design by designer for Dark-Mode and Light-Mode

I created a custom toolbar for the survey, please check the layout file. The progressbar drawable in this toolbar is also customized, per the survey mocks. See how it looks in the light mode screenshots with the white outline.

Also I think we need to introduce NPS Survey layout for landscape version because you can see the output, screen is not scrollable like welcome screen is incomplete and cutting from bottom:

Yes, that is correct.

MohitGupta121

@adhiamboperes PTAL, here are the screenshots of the progress bar in both light and dark-mode. Thanks!

Light Mode	Dark Mode

adhiamboperes · 2023-07-25T20:05:30Z

@adhiamboperes PTAL, here are the screenshots of the progress bar in both light and dark-mode. Thanks!

Light Mode Dark Mode

Thank you, this LGTM

MohitGupta121 · 2023-07-25T20:40:02Z

Thank you so much @adhiamboperes anything else I need to update in this PR?

adhiamboperes · 2023-07-28T00:11:06Z

Thank you so much @adhiamboperes anything else I need to update in this PR?

Hey @MohitGupta121, I have opened a PR for the landscape layouts here and assigned it to you and @kkmurerwa for review. If you are happy with it, we will merge it into this branch before merging to develop.

oppiabot · 2023-07-31T22:18:01Z

Assigning @seanlip for code owner reviews. Thanks!

adhiamboperes

Left a couple of minor follow up comments

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

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

…_survey_dark_mode

MohitGupta121 · 2023-07-31T22:47:18Z

@adhiamboperes done that separate dimensions changes for survey. Thanks!

seanlip · 2023-08-01T03:22:24Z

@MohitGupta121 I think this looks great. Just one question: do we use dark green text on black anywhere else in the app (e.g. "Previous" on the black background)? I find that hard to read, but it's probably fine to leave as-is if it's an existing design pattern for dark mode (i.e. we've used it in other places too).

Thanks!

MohitGupta121 · 2023-08-01T03:46:44Z

@seanlip PTAL, Thanks.
Here are some comparison:

Current Green	Oppia Primary Green	Dark Green

seanlip · 2023-08-01T08:45:26Z

@MohitGupta121 For me, the leftmost one is the most visible, but it is still a little dark against the background.

What is the colour contrast ratio and does it meet accessibility standards? That's one way to test.

MohitGupta121 · 2023-08-01T15:46:09Z

@seanlip Sorry, yes you are right. This does not match accessibility standards. Here is the output:

MohitGupta121 · 2023-08-01T15:53:40Z

@seanlip After changing the color to oppia_bright_green here is the output screenshot and accessibility result:

Screenshot:

Accessibility Result

new.record.webm

seanlip

Thanks @MohitGupta121 ! No further concenrs.

oppiabot · 2023-08-01T19:11:04Z

Hi @MohitGupta121, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

adhiamboperes · 2023-08-01T19:12:45Z

Thanks @MohitGupta121, @seanlip, @kaiyuxu.

I'm merging this now.

Implement dark mode for nps survey

dd32a17

MohitGupta121 requested a review from adhiamboperes July 25, 2023 06:44

MohitGupta121 requested a review from a team as a code owner July 25, 2023 06:44

MohitGupta121 assigned adhiamboperes Jul 25, 2023

MohitGupta121 commented Jul 25, 2023

View reviewed changes

adhiamboperes requested changes Jul 25, 2023

View reviewed changes

oppiabot bot unassigned adhiamboperes Jul 25, 2023

oppiabot bot assigned MohitGupta121 Jul 25, 2023

Merge branch 'develop' into nps_survey_dark_mode

b31b18f

MohitGupta121 requested a review from adhiamboperes July 25, 2023 14:19

MohitGupta121 assigned adhiamboperes and unassigned MohitGupta121 Jul 25, 2023

adhiamboperes assigned MohitGupta121 and unassigned adhiamboperes Jul 25, 2023

MohitGupta121 and others added 2 commits July 25, 2023 13:45

progress bar survey dark-mode

d7da9e8

Merge branch 'develop' into nps_survey_dark_mode

400eab5

MohitGupta121 assigned adhiamboperes and unassigned MohitGupta121 Jul 25, 2023

MohitGupta121 commented Jul 25, 2023

View reviewed changes

adhiamboperes assigned kaiyuxu and unassigned adhiamboperes Jul 26, 2023

Merge branch 'develop' into nps_survey_dark_mode

bf455b0

adhiamboperes mentioned this pull request Jul 28, 2023

Fix Part of #5086: Add Layout Land for Survey UI #5110

Merged

6 tasks

oppiabot bot assigned seanlip Jul 31, 2023

adhiamboperes reviewed Jul 31, 2023

View reviewed changes

app/src/main/res/drawable/nps_survey_button_background.xml Outdated Show resolved Hide resolved

app/src/main/res/drawable/nps_survey_button_inactive_background.xml Outdated Show resolved Hide resolved

MohitGupta121 added 2 commits July 31, 2023 16:44

make separate dimension for survey

d92c8c9

Merge remote-tracking branch 'upstream/nps_survey_dark_mode' into nps…

9320ddb

…_survey_dark_mode

adhiamboperes approved these changes Jul 31, 2023

View reviewed changes

seanlip assigned MohitGupta121 and unassigned seanlip Aug 1, 2023

MohitGupta121 assigned seanlip and unassigned MohitGupta121 Aug 1, 2023

seanlip assigned MohitGupta121 and unassigned seanlip Aug 1, 2023

MohitGupta121 assigned seanlip and unassigned MohitGupta121 Aug 1, 2023

fix night color of Previous button

2b7919e

seanlip approved these changes Aug 1, 2023

View reviewed changes

seanlip assigned MohitGupta121 and unassigned seanlip Aug 1, 2023

oppiabot bot added the PR: LGTM label Aug 1, 2023

adhiamboperes merged commit a888729 into develop Aug 1, 2023
36 checks passed

adhiamboperes deleted the nps_survey_dark_mode branch August 1, 2023 19:12

adhiamboperes mentioned this pull request Sep 1, 2023

Fix #4735 Text unit is set in dp instead of sp #5134

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #5086 : Implement Dark Mode support for NPS Survey #5104

Fix #5086 : Implement Dark Mode support for NPS Survey #5104

MohitGupta121 commented Jul 25, 2023 •

edited

Loading

MohitGupta121 left a comment

adhiamboperes left a comment

oppiabot bot commented Jul 25, 2023

oppiabot bot commented Jul 25, 2023

MohitGupta121 commented Jul 25, 2023 •

edited

Loading

adhiamboperes commented Jul 25, 2023

MohitGupta121 left a comment •

edited

Loading

adhiamboperes commented Jul 25, 2023

MohitGupta121 commented Jul 25, 2023

adhiamboperes commented Jul 28, 2023

oppiabot bot commented Jul 31, 2023

adhiamboperes left a comment

MohitGupta121 commented Jul 31, 2023 •

edited

Loading

seanlip commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023 •

edited

Loading

seanlip commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023

seanlip left a comment

oppiabot bot commented Aug 1, 2023

adhiamboperes commented Aug 1, 2023

Fix #5086 : Implement Dark Mode support for NPS Survey #5104

Fix #5086 : Implement Dark Mode support for NPS Survey #5104

Conversation

MohitGupta121 commented Jul 25, 2023 • edited Loading

Explanation

Essential Checklist

For UI-specific PRs only

NPS Survey Welcome Screen

NPS Survey Question screen (Button Disabled)

NPS Survey Question screen (Button Enabled)

NPS Survey Score Screen

NPS Survey Final comment screen (Button Enabled)

NPS Survey Final comment screen (Button Disabled)

NPS Survey Thanks You screen

NPS Survey Exit Dialog

MohitGupta121 left a comment

Choose a reason for hiding this comment

adhiamboperes left a comment

Choose a reason for hiding this comment

oppiabot bot commented Jul 25, 2023

oppiabot bot commented Jul 25, 2023

MohitGupta121 commented Jul 25, 2023 • edited Loading

adhiamboperes commented Jul 25, 2023

MohitGupta121 left a comment • edited Loading

Choose a reason for hiding this comment

adhiamboperes commented Jul 25, 2023

MohitGupta121 commented Jul 25, 2023

adhiamboperes commented Jul 28, 2023

oppiabot bot commented Jul 31, 2023

adhiamboperes left a comment

Choose a reason for hiding this comment

MohitGupta121 commented Jul 31, 2023 • edited Loading

seanlip commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023 • edited Loading

seanlip commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023

MohitGupta121 commented Aug 1, 2023

Screenshot:

Accessibility Result

seanlip left a comment

Choose a reason for hiding this comment

oppiabot bot commented Aug 1, 2023

adhiamboperes commented Aug 1, 2023

MohitGupta121 commented Jul 25, 2023 •

edited

Loading

MohitGupta121 commented Jul 25, 2023 •

edited

Loading

MohitGupta121 left a comment •

edited

Loading

MohitGupta121 commented Jul 31, 2023 •

edited

Loading

MohitGupta121 commented Aug 1, 2023 •

edited

Loading