Skip to content
This repository has been archived by the owner on Oct 4, 2022. It is now read-only.

Add focus style to wizard navigation buttons #8

Merged
merged 5 commits into from
Apr 23, 2019

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Mar 8, 2019

Supersedes Yoast/yoast-components#834

Summary

This PR can be summarized in the following changelog entry:

  • [configuration-wizard] Adds focus style to the Configuration Wizard navigation buttons

Relevant technical choices:

  • adds a basic focus style
  • adds a new #5b9dd9 blue color to the palette: this is the color used by WordPress for focus
  • to clarify: in Chrome you will see the native webkit focus style, but that's not what we want. Also, other browsers have different native focus styles: we want our own style

Test instructions

  • go in the configuration wizard in the standalone app
  • tab with the keyboard to the "previous" and "next" buttons
  • see there's a focus style

screenshot 2019-02-27 at 14 39 30

screenshot 2019-02-27 at 14 53 08

Note:
while working on this I've noticed the state of the CSS could be improved. There are duplications of rules between the plugin and the yoast-components. The preview of the Wizard in the standalone app looks different from the Wizard in the plugin. This doesn't help in testing. Also, there shouldn't be hardcoded colors in the yoast-components (only colors from the palette should be used).
Will create a separate issue.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Fixes Yoast/yoast-components#833

@afercia afercia added the UI change PRs that result in a change in the UI label Mar 8, 2019
diedexx pushed a commit that referenced this pull request Mar 11, 2019
Bump the ECMAScript version to 2017
@johannadevos
Copy link
Contributor

@afercia When merging develop into your branch, I can see that 3 out of 5 files in your PR have been deleted in the develop branch in the meantime:

Screenshot 2019-04-18 at 12 39 20

What used to be /packages/yoast-components/composites/OnboardingWizard/onboarding-wizard.scss has become /packages/configuration-wizard/src/configuration-wizard.scss. I'm afraid that you'll have to implement your changes in this new file before code review / acceptance test can continue. In other words, can you resolve the merge conflicts that have arisen between your branch and develop?

@afercia
Copy link
Contributor Author

afercia commented Apr 18, 2019

Solved merge conflicts.

Also, I've improved the focus style trying to better respect the original design. Right now, these "links" (actually they're buttons) have a bottom border to emulate a link style. My previous change removed the bottom border in favor of text-underline but that made the underline very close to the text. See previous screenshots.

Now, I'm using a different CSS to make this line more spaced from the text. In the screenshot below: normal and focused:

Screenshot 2019-04-18 at 15 16 12

The fourth commit is only for coding standards, you can ignore it and check only the third one.
The .scss file used to use tab characters for indentation, see for example https://github.com/Yoast/javascript/blob/yoast-social-previews%401.4.3-alpha.0/packages/yoast-components/composites/OnboardingWizard/onboarding-wizard.scss
This was somehow lost while moving/restoring files in the new monorepo and the file ended up with using spaces. The fourth commit restores tabs.

@hwinne
Copy link
Contributor

hwinne commented Apr 19, 2019

CR Done 👍

@abotteram
Copy link
Contributor

Acceptance 👍

@abotteram abotteram added this to the 11.2/yoastseo milestone Apr 23, 2019
@abotteram abotteram merged commit f79dfae into develop Apr 23, 2019
@abotteram abotteram deleted the 833-wizard-buttons-focus-style branch April 23, 2019 08:20
@afercia afercia mentioned this pull request Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
UI change PRs that result in a change in the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wizard previous and next buttons focus style
5 participants