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

Issue/update color scheme #9495

Merged
merged 170 commits into from
Apr 8, 2019
Merged

Issue/update color scheme #9495

merged 170 commits into from
Apr 8, 2019

Conversation

theck13
Copy link
Contributor

@theck13 theck13 commented Mar 29, 2019

Fix

Update the color scheme to Classic Bright to match the color changes described in https://en.blog.wordpress.com/2019/01/08/customize-your-wordpress-com-dashboard. These changes include updating color values, refactoring color names, renaming file names based on the new color values/names, and removing unused colors. There are no functional changes.

Test

An alpha build can be downloaded from here. It will be installed alongside the production build (i.e. two WordPress apps will be on the device; one production and one debug) so changes can be compared on the same device.

Review

The code review should not worry about the color names or specific changes to the color of the interface, but it should check that there are no breaking changes. The design and interface have been reviewed multiple times by @mattmiklic, but it should still be reviewed again to ensure nothing was missed.

Release

The RELEASE-NOTES.txt document was updated in 443b2de with the following:

Updated color scheme to Classic Bright

theck13 added 30 commits March 5, 2019 12:28
Copy link
Member

@mattmiklic mattmiklic left a comment

Choose a reason for hiding this comment

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

Everything looks good, I think this incorporates everything we've decided to include in this v1. 👍

@0nko 0nko self-assigned this Apr 2, 2019
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

I've glanced over the code and tested different parts of the app and I haven't noticed any broken functionality, but there are a few places where the old colors are still used (all screens of the login flow).

Notice the text box and button color:
image
image

In other parts of the app, the new highlight blue is used:
image

The selected menu item background uses the old blue (unlike Woo, which uses the grey color):
image

The main menu selection is grey:
image

Also, I'm not sure if the toolbar selected background color is correct:
image

The Activity Log page modification icon color is off (should be white):
image

@theck13
Copy link
Contributor Author

theck13 commented Apr 4, 2019

Thanks for the review and catching those bugs, @0nko!

I added resources to override the login library colors in a9ba165. See the screenshots below for illustration.

color_login_button_input

color_login_progress

You'll notice the button color was updated to accent (#d52c82) also. That's a pleasant side effect since we were going to update the buttons to that color in a subsequent pull request anyway. @mattmiklic, can you confirm that's desired behavior?

I removed the custom selectable background drawable in af1657a. See the screenshots below for illustration.

color_pages_menu

I think the toolbar touch feedback color is correct since the app theme's parent style is Theme.AppCompat.Light.DarkActionBar. I checked other apps with a dark toolbar and light text/actions. They had the same touch feedback. See the screenshots below for illustration.

color_toolbar_feedback

I'm not sure why the page modification icon color shows up as dark yellow, which I think is warning_700 (#bd9219), rather than white on your device. It is shown as white when I tested it. See the screenshot below for illustration.

color_activity_modified

Can you share how you got to that screen in case there are certain steps to reproduce it?

@mattmiklic
Copy link
Member

Thanks for catching those spots I missed, @0nko!

@mattmiklic, can you confirm that's desired behavior?

Yes, that's perfect, the pink buttons look great there.

I noticed in your before/after login screenshot that I think the "log in with google" and "log in by entering your site address" links are maybe still using the old blue -- if so we should change those to primary blue.

@theck13
Copy link
Contributor Author

theck13 commented Apr 4, 2019

I noticed in your before/after login screenshot that I think the "log in with google" and "log in by entering your site address" links are maybe still using the old blue -- if so we should change those to primary blue.

I added color resources for the login button text color in 6e4bc09. See the screenshots below for illustration.

color_login_button_text

I updated the background color for shortcuts too in order to follow the guidelines here. See the screenshots below for illustration.

color_shortcuts

@mattmiklic
Copy link
Member

Good catch on the shortcuts. Those links now look good. 👍

@0nko
Copy link
Contributor

0nko commented Apr 8, 2019

Thanks for the changes, @theck13. It looks great 👍.

@0nko 0nko merged commit fa4f422 into develop Apr 8, 2019
@0nko 0nko deleted the issue/update-color-scheme branch April 8, 2019 10:34
@theck13 theck13 removed [Status] Needs Code Review [Status] Needs Design Review A designer needs to sign off on the implemented design. labels Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants