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

Dark mode implementation for AdministratorControls, Help, Options, ProfileReset, ProfileRename, AppVersion, ProfileList, ProfileEdit etc #4795

Merged
merged 19 commits into from
Dec 21, 2022
Merged

Dark mode implementation for AdministratorControls, Help, Options, ProfileReset, ProfileRename, AppVersion, ProfileList, ProfileEdit etc #4795

merged 19 commits into from
Dec 21, 2022

Conversation

rt4914
Copy link
Contributor

@rt4914 rt4914 commented Dec 19, 2022 •
edited
Loading

Explanation

This PR fixes dark mode for following activities:

OptionsActivity

  • ReadingTextSizeActivity
  • AudioLanguageActivity
  • AppLanguageActivity

HelpActivity

  • FAQListActivity
  • FAQSingleActivity
  • ThirdPartyDependencyActivity
  • LicenseListActivity
  • LicenseViewerActivity
  • PolicyViewerActivity

AdministratorControlsActivity

  • AppVersionActivity
  • ProfilListActivity
  • ProfilEditActivity
  • ProfileRenameActivity
  • ProfileResetPinActivity

There were a lot of issues with mocks, for example:

  • Mocks were not available for reset, rename screens in light-mode portrait mobile
  • Dark mode mocks were not available for some internal screens
  • Dark mode mocks were not available for tablet multi-pane screens.
  • There were inconsistencies in designs of similar screen.

This PR makes sure that all the screens support dark-mode and also the components across various screens should have consistent design guidelines.

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

Options Activity and its internal screens

Before After
Screenshot_20221221_113351 Screenshot_20221221_113418
Screenshot_20221221_113357 Screenshot_20221221_113424
Screenshot_20221221_113403 Screenshot_20221221_113431
Screenshot_20221221_113836 Screenshot_20221221_113853
Screenshot_20221221_113842 Screenshot_20221221_113859

Help Activity and its internal screens

Before After
Screenshot_20221221_114027 Screenshot_20221221_114152
Screenshot_20221221_114034 Screenshot_20221221_114157
Screenshot_20221221_114039 Screenshot_20221221_114203
Screenshot_20221221_114045 Screenshot_20221221_114210
Screenshot_20221221_114051 Screenshot_20221221_114214
Screenshot_20221221_114100 Screenshot_20221221_114221
Screenshot_20221221_114110 Screenshot_20221221_114227
Screenshot_20221221_114114 Screenshot_20221221_114234
Screenshot_20221221_114615 Screenshot_20221221_114724
Screenshot_20221221_114621 Screenshot_20221221_114728
Screenshot_20221221_114627 Screenshot_20221221_114731
Screenshot_20221221_114632 Screenshot_20221221_114736
Screenshot_20221221_114641 Screenshot_20221221_114741
Screenshot_20221221_114649 Screenshot_20221221_114749
Screenshot_20221221_114656 Screenshot_20221221_114754

Administrator Controls and its internal activities

Before After
Screenshot_20221221_115905 Screenshot_20221221_115954
Screenshot_20221221_115912 Screenshot_20221221_120004
Screenshot_20221221_115919 Screenshot_20221221_120009
Screenshot_20221221_120012 Screenshot_20221221_115925
Screenshot_20221221_115929 Screenshot_20221221_120018
Screenshot_20221221_120022 Screenshot_20221221_115934
Screenshot_20221221_120404 Screenshot_20221221_120449
Screenshot_20221221_120411 Screenshot_20221221_120455
Screenshot_20221221_120420 Screenshot_20221221_120500
Screenshot_20221221_120426 Screenshot_20221221_120505
Screenshot_20221221_120433 Screenshot_20221221_120510

@MohitGupta121 MohitGupta121 self-requested a review December 20, 2022 12:10
@rt4914 rt4914 self-assigned this Dec 20, 2022
MohitGupta121
MohitGupta121 requested changes Dec 20, 2022
View reviewed changes
Copy link
Member

@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.

@rt4914 PTAL

@rt4914 rt4914 marked this pull request as ready for review December 21, 2022 06:47
@rt4914
Copy link
Contributor Author

rt4914 commented Dec 21, 2022

Me and @MohitGupta121 did review this PR over a 2 hr call where we went through each and every view, compared it with mocks and if something was changed w.r.t to mocks then if both of us agreed we finalised that and applied that change.

Its not possible to review this PR using mocks as they are inconsistent and this PR does require a lot of context.

@rt4914 rt4914 assigned MohitGupta121 and unassigned rt4914 Dec 21, 2022
MohitGupta121
MohitGupta121 approved these changes Dec 21, 2022
View reviewed changes
Copy link
Member

@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.

@rt4914 LGTM, Thanks.

@MohitGupta121 MohitGupta121 self-requested a review December 21, 2022 09:51
@oppiabot
Copy link

oppiabot bot commented Dec 21, 2022

Unassigning @MohitGupta121 since they have already approved the PR.

MohitGupta121
MohitGupta121 approved these changes Dec 21, 2022
View reviewed changes
Copy link
Member

@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.

@rt4914 I not find any open issues related to these screens in the dark-mode-project-board. So I think there is no need to link any issue.

@rt4914 rt4914 assigned seanlip and unassigned rt4914 Dec 21, 2022
@rt4914 rt4914 requested a review from seanlip December 21, 2022 11:11
@rt4914
Copy link
Contributor Author

rt4914 commented Dec 21, 2022

@seanlip With reference to this can you approve this PR?

seanlip
seanlip approved these changes Dec 21, 2022
View reviewed changes
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Confirming this looks good from a UI perspective. Thanks!

@rt4914 rt4914 merged commit 1a0b7ef into develop Dec 21, 2022
@rt4914 rt4914 deleted the dark-mode-admin-options-and-help branch December 21, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MohitGupta121 MohitGupta121 MohitGupta121 approved these changes

@seanlip seanlip seanlip approved these changes

Assignees

@seanlip seanlip

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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