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

Security: update notice styles in backup section #23487

Merged
merged 1 commit into from
Mar 22, 2018

Conversation

simison
Copy link
Member

@simison simison commented Mar 21, 2018

  1. Use Notice component instead of custom styles to match with other Calypso notifications, updated in Update notice colors to pass WCAG AA guidelines #17796

  2. Use SectionHeader component for the header instead of CompactCard (works better for alignment)

There's a similar PR in Jetpack open: Automattic/jetpack#9080

The section below the header has some style issues in mobile; I'll address those in a separate PR.

Screenshots (before & after)

Mobile

screen shot 2018-03-21 at 10 54 46
screen shot 2018-03-21 at 10 54 57

Desktop

screen shot 2018-03-21 at 11 29 54
screen shot 2018-03-21 at 11 30 00

Testing

  1. Have a site connected to Rewind
  2. Go to https://wordpress.com/settings/security/:site and see that the section works and looks good in different screen sizes.

1) Refactor header to use `Notice` component instead of custom styles:
  - Improves accessibility by following WCAG AA guidelines for contrast ratio.
  - Matches with other Calypso notifications, updated in #17796

2) Use `SectionHeader` component for positioning elements
@simison simison added [Feature] Site Settings All other general site settings. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR Security labels Mar 21, 2018
@matticbot
Copy link
Contributor

@MichaelArestad
Copy link
Contributor

I love this. Nice work. Ship it!

@MichaelArestad MichaelArestad added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 21, 2018
@simison simison removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Mar 21, 2018
@simison simison merged commit 617b0bb into master Mar 22, 2018
@simison simison deleted the update/security-backups-notice branch March 22, 2018 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings. Security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants