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

Apply consistent admin theme colors to common UI components #2059

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

eason9487
Copy link
Member

@eason9487 eason9487 commented Aug 17, 2023

Changes proposed in this Pull Request:

Closes #2058

This PR applies the admin theme colors for:

  • The links within the unsupported notices on the Get Started page.
  • The active dot icon of the page control in Guide component.
  • The chevron icon in TopBar component.

In addition, this PR also simplifies the layout and style of TopBar component.

💡 The color of native <select> won't be fixed in this PR as it's specified in WP core.

Screenshots:

📷 Link color on the Get Started page

image

📷 Active dot in the page control

image

📷 Icon color when hovering

Kapture 2023-08-17 at 15 52 50

Detailed test instructions:

Locally change the value of mcSupportedCountry, mcSupportedLanguage, and adsSetupComplete to false:

'mcSupportedCountry' => $this->merchant_center->is_store_country_supported(),
'mcSupportedLanguage' => $this->merchant_center->is_language_supported(),
'adsCampaignConvertStatus' => $this->options->get( OptionsInterface::CAMPAIGN_CONVERT_STATUS ),
'adsSetupComplete' => $this->ads->is_setup_complete(),

  1. Use a WP version >= 6.1
  2. Install a nightly build WooCommerce to have the latest color fixes in WC core
  3. Go to WP Admin > Users > Profile page: /wp-admin/profile.php
    • In the Admin Color Scheme setting, select an option rather than "Default", such as "Modern"
    • Click the "Update Profile" button at the bottom to save the change
  4. Go to the Get Started page
    • Check the color of links within the unsupported notices
  5. Go to this page: /wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fproduct-feed&guide=submission-success
    • Check the color of the active dot icon of the page control
  6. Go to the Edit Free Listings page
    • Over the chevron icon at the top-left corner to check its :hover color
    • Check if the TopBar component layout is rendered as it was before

Changelog entry

Tweak - Apply consistent admin theme colors to common UI components.

@eason9487 eason9487 requested a review from a team August 17, 2023 08:05
@eason9487 eason9487 self-assigned this Aug 17, 2023
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Aug 17, 2023
Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Hey @eason9487, thanks for improving the colours used throughout GLA 👍

Tested locally and following your testing instructions all relevant colours were updated to match the admin selection. There are two areas where the hard-coded colour is still coming through.

Unchecked requirements on the pre-launch list:

Screenshot 2023-08-18 at 11 24 13

Both final URL icons on the Optimize your campaign screen:

Screenshot 2023-08-18 at 11 21 31

@eason9487
Copy link
Member Author

Hi @martynmjones, thanks for the review.

Unchecked requirements on the pre-launch list: [...]

Screenshot 2023-08-18 at 11 24 13

Thanks for catching this! Fixed in fcb0dcd.

Both final URL icons on the Optimize your campaign screen:

Screenshot 2023-08-18 at 11 21 31

These two I didn't include because they are non-interactive icons and are paired with other colors in it or the background color where it is used.


Could you help with a new round of code reviews?

Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

These two I didn't include because they are non-interactive icons and are paired with other colors in it or the background color where it is used.

That makes sense and thanks for the clarification 👍

Tested again and the issue with the pre-launch checklist has been fixed so LGTM ✅

@eason9487 eason9487 merged commit aa2a060 into develop Aug 21, 2023
4 checks passed
@eason9487 eason9487 deleted the tweak/2058-use-admin-theme-color branch August 21, 2023 03:39
@eason9487 eason9487 mentioned this pull request Aug 22, 2023
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use admin theme colors instead of hardcoded colors for common UI components
2 participants