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.

Sign up for GitHub

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

Jump to bottom

Fixed Part #4200 : Added dark mode support to AudioLanguageActivity, ReadingTextSizeActivity, AppLanguageActivity #4415

Merged
merged 28 commits into from
Aug 24, 2022
Merged

Fixed Part #4200 : Added dark mode support to AudioLanguageActivity, ReadingTextSizeActivity, AppLanguageActivity #4415

merged 28 commits into from
Aug 24, 2022

Conversation

MohitGupta121
Copy link
Member

@MohitGupta121 MohitGupta121 commented Jul 2, 2022 •
edited
Loading

Fixed One Part of #4200 : Added dark mode support to AudioLanguageActivity, ReadingTextSizeActivity, AppLanguageActivity

Dark Mode Mock:-

Default Dark Mode
Screenshot_2022-05-31-12-23-27-332_com android camera
Default Dark Mode
Default Dark Mode
Screenshot_2022-05-31-12-23-27-332_com android camera

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

@MohitGupta121 MohitGupta121 requested a review from rt4914 as a code owner July 2, 2022 15:20
@MohitGupta121 MohitGupta121 self-assigned this Jul 2, 2022
MohitGupta121
MohitGupta121 commented Jul 2, 2022
View reviewed changes
Copy link
Member Author

@MohitGupta121 MohitGupta121 left a comment

Choose a reason for hiding this comment

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

Done Dark Mode

@MohitGupta121 MohitGupta121 assigned rt4914 and unassigned MohitGupta121 Jul 5, 2022
rt4914
rt4914 suggested changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@MohitGupta121 PTAL

app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-xxhdpi/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/component_colors.xml Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned MohitGupta121 and unassigned rt4914 Jul 11, 2022
MohitGupta121
MohitGupta121 commented Jul 12, 2022
View reviewed changes
Copy link
Member Author

@MohitGupta121 MohitGupta121 left a comment

Choose a reason for hiding this comment

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

Done with requested changes

@MohitGupta121 MohitGupta121 requested a review from rt4914 July 12, 2022 04:08
@BenHenning BenHenning self-assigned this Aug 16, 2022
@BenHenning
Copy link
Member

As discussed @MohitGupta121 can you upload new screenshots that use a dark mode emulator? It may be something odd with the physical device or specific Android OEM version (i.e. maybe it's performing some sort of extra shading).

@MohitGupta121
Copy link
Member Author

Okay, I'll update it in a few hours.

@MohitGupta121
Copy link
Member Author

@BenHenning @rt4914 I updated the screenshots through emulator dark mode. Can you please review it now. Is it okay?

@BenHenning
Copy link
Member

Thanks @MohitGupta121. To confirm: do the individual rows appear as dark on the emulator as they do in the screenshots you added?

@MohitGupta121
Copy link
Member Author

MohitGupta121 commented Aug 19, 2022 •
edited
Loading

Thanks @MohitGupta121. To confirm: do the individual rows appear as dark on the emulator as they do in the screenshots you added?

Yes they all are the same in the emulator. It's look same as in dark mode mockups.

rt4914
rt4914 suggested changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

Nice work @MohitGupta121 Just one suggestion.

app/src/main/res/values/component_colors.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned MohitGupta121 and unassigned rt4914 Aug 24, 2022
@MohitGupta121
Copy link
Member Author

MohitGupta121 commented Aug 24, 2022 •
edited
Loading

Suddenly 2 test case are failing. Unable to find exact reason. I only added EOF. It's due to this https://github.com/oppia/oppia-android/runs/8001170895?check_suite_focus=true#step:7:1764

@MohitGupta121 MohitGupta121 requested a review from rt4914 August 24, 2022 19:26
rt4914
rt4914 approved these changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

@rt4914
Copy link
Contributor

rt4914 commented Aug 24, 2022

@MohitGupta121 Please don't Resolve Conversations. The conversation should be resolved by the person who started that thread.

@rt4914 rt4914 merged commit 2ff2945 into oppia:develop Aug 24, 2022
@MohitGupta121
Copy link
Member Author

@MohitGupta121 Please don't Resolve Conversations. The conversation should be resolved by the person who started that thread.

Oh okay 😅 sorry once again take care next time.

rt4914 pushed a commit that referenced this pull request Aug 29, 2022
@MohitGupta121
Fixed Part-2 #4200 : Added dark mode support to Alert Dialog Views #4415
afdddba 
 (#4534)

* fixed text cut off

* fix text cutoff in all layouts

* add proper naming

* implement darkmode AudioLanguage,ReadingTextSize,AppLanguage Activities

* minor fixes

* develop updated

* minor changes

* fixed build error

* Tests Failed Fixed

* Update color_defs.xml

* added EOL

* alert dialog dark mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

Assignees

@rt4914 rt4914

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MohitGupta121 @rt4914 @BenHenning