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.

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

Fix #4176: Added dark mode support to profile activities and app version activity #4549

Conversation

aayushimathur6
Copy link
Collaborator

@aayushimathur6 aayushimathur6 commented Sep 2, 2022

Explanation

Fixes #4176

Profile List: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/ddc3dadf-10e4-45f6-aeea-eb8a330e00ff/specs/

Profile Edit: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/693301de-5907-4f6a-b183-037412478267/

Profile Rename: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/d0227921-1c24-4bb7-8667-a8fa5f4742dd/

Profile Reset: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/32c9276d-072c-4edf-abf9-435323bb475a/

App Version: https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/79e0846d-de5c-42b7-93aa-f93d4995616d/

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
BEFORE AFTER
Screenshot_20220916_153310 Screenshot_20220916_152556
Screenshot_20220902_164755 Screenshot_20220902_165035
Screenshot_20220916_153741 Screenshot_20220916_152840
Screenshot_20220916_153924 Screenshot_20220916_152923
Screenshot_20220916_154020 Screenshot_20220916_153037

@aayushimathur6
Copy link
Collaborator Author

@rt4914 made a new pull request for issue no. 4176 (#4176), PTAL thanks

@oppiabot oppiabot bot assigned rt4914 Sep 2, 2022
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.

@aayushimathur6 Suggested changes.

app/src/main/res/layout/profile_edit_fragment.xml Outdated Show resolved Hide resolved
app/src/main/res/values-night/color_palette.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned aayushimathur6 and unassigned rt4914 Sep 7, 2022
@aayushimathur6
Copy link
Collaborator Author

@rt4914 PTAL

@aayushimathur6
Copy link
Collaborator Author

@rt4914 also for the one check failing then do I need to add any regex pattern?

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.

Screenshot 2022-09-15 at 1 05 16 AM

In this and similar deactivated-save-button the border color does not match with that of mocks. It should be dull https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/32c9276d-072c-4edf-abf9-435323bb475a/specs/

Also, have a look at this to understand how to use 8-characters color code correctly
https://github.com/oppia/oppia-android/wiki/Working-on-UI#color-code

@rt4914 rt4914 assigned aayushimathur6 and unassigned rt4914 Sep 14, 2022
@aayushimathur6
Copy link
Collaborator Author

@rt4914 corrected the color, PTAL

@oppiabot oppiabot bot assigned rt4914 and unassigned aayushimathur6 Sep 15, 2022
@oppiabot
Copy link

oppiabot bot commented Sep 15, 2022

Unassigning @aayushimathur6 since a re-review was requested. @aayushimathur6, please make sure you have addressed all review comments. Thanks!

@aayushimathur6
Copy link
Collaborator Author

Screenshot 2022-09-15 at 1 05 16 AM

In this and similar deactivated-save-button the border color does not match with that of mocks. It should be dull https://xd.adobe.com/view/c05e9343-60f6-4c11-84ac-c756b75b940f-950d/screen/32c9276d-072c-4edf-abf9-435323bb475a/specs/

Also, have a look at this to understand how to use 8-characters color code correctly https://github.com/oppia/oppia-android/wiki/Working-on-UI#color-code

done

@aayushimathur6
Copy link
Collaborator Author

@rt4914 updated the content cell as well with the lightening boundary color of the save button according to the android working on UI wiki

rt4914
rt4914 previously requested changes Sep 21, 2022
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.

@aayushimathur6 Merged with latest develop.

Also, I do think that the border of disabled button is quite bright as compared to mocks. Can you please check why is that, maybe expermient with colors a bit, like trying
#3BD1C466 and #663BD1C4 and seeing the difference.

app/src/main/res/values/component_colors.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned aayushimathur6 and unassigned rt4914 Sep 21, 2022
@aayushimathur6
Copy link
Collaborator Author

aayushimathur6 commented Sep 26, 2022

@aayushimathur6 Merged with latest develop.

Also, I do think that the border of disabled button is quite bright as compared to mocks. Can you please check why is that, maybe expermient with colors a bit, like trying #3BD1C466 and #663BD1C4 and seeing the difference.

Done with merging
for this @rt4914 earlier I was applying
image
this which is too bright i.e #3BD1C4
but after you shared the mock I applied the light version i.e #663BD1C4 but this is named here previously as bright
image
still, the check is failing
Also, for #3BD1C466 this is not the color that I have to apply you can see the following
image
I am not getting the exact color

@aayushimathur6
Copy link
Collaborator Author

@rt4914 @BenHenning PTAL

@oppiabot oppiabot bot assigned BenHenning and rt4914 and unassigned aayushimathur6 Oct 2, 2022
@oppiabot
Copy link

oppiabot bot commented Oct 2, 2022

Unassigning @aayushimathur6 since a re-review was requested. @aayushimathur6, please make sure you have addressed all review comments. Thanks!

@BenHenning
Copy link
Sponsor Member

Sorry, will need to look at this tomorrow.

… Add-dark-mode-support-to-Profile-activities-and-AppVersion-activty
Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @aayushimathur6. I didn't have too much to comment on, and I checked past comments from Rajat.

PTAL at my follow-up comment and also make sure that CI checks are passing before sending this back.

@aayushimathur6
Copy link
Collaborator Author

Thanks @aayushimathur6. I didn't have too much to comment on, and I checked past comments from Rajat.

PTAL at my follow-up comment and also make sure that CI checks are passing before sending this back.

I have looked at your comment I was making a silly mistake, All checks have passed.
PTAL @BenHenning

@oppiabot
Copy link

oppiabot bot commented Oct 7, 2022

Unassigning @aayushimathur6 since a re-review was requested. @aayushimathur6, please make sure you have addressed all review comments. Thanks!

Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @aayushimathur6! Approved.

@BenHenning BenHenning dismissed rt4914’s stale review October 10, 2022 23:33

Rajat is out currently, and I've verified that his requested changes have been addressed.

@BenHenning BenHenning changed the title added dark mode support to profile activities and app version activity Fix #4176: Added dark mode support to profile activities and app version activity Oct 10, 2022
@BenHenning BenHenning merged commit d192258 into oppia:develop Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants