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

[EuiSteps] Updates for Amsterdam and added disabled and loading states #4338

Merged
merged 9 commits into from
Dec 7, 2020

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Dec 3, 2020

Also updated the docs a bit too.

Added disabled as a status for EuiStep

Closes #3339

Screen Shot 2020-12-03 at 15 10 14 PM

Added loading as a status for EuiStep

Closes #4322

Screen Shot 2020-12-03 at 15 11 46 PM

Updated Amsterdam styling

We aligned the styles between Vertical and Horizontal steps and removed center line indicators.

The design

image

BEFORE

image

AFTER

image


Some notes

  1. A few of the props are then obsolete in Amsterdam as they're overridden by the overall style of the status. I've marked these as **DEPRECATED IN AMSTERDAM**.
  2. The previous design would not render the number if the step is incomplete, while the new one does. I ended up changing this to always render the number but hide visually in the default theme.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs
  • Added documentation
  • [ ] Checked Code Sandbox works for the any docs examples
  • Added or updated jest tests
  • [ ] Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@cchaos cchaos requested a review from elizabetdev December 3, 2020 20:29
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4338/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4338/

@snide
Copy link
Contributor

snide commented Dec 4, 2020

Minor visual comment. Do you think about putting the loading ring on the step-circle itself, rather than inside the circle?

warning: 'euiStepNumber--warning',
danger: 'euiStepNumber--danger',
disabled: 'euiStepNumber--disabled',
complete: 'euiStepNumber--complete',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add selected as a status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The selected status isn't actually any different from the default (blue background, white number). So there would be nothing to change here.

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

I looked at the code and tested it locally in Safari, Chrome, Edge, and Firefox, and LGTM. 🎉

+1 for what @snide is suggesting:

Do you think about putting the loading ring on the step-circle itself

@cchaos
Copy link
Contributor Author

cchaos commented Dec 7, 2020

Originally I had thought it would clash with the connecting lines which is why I had it inside, but I tried it out and it doesn't 👍 !

image

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4338/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4338/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4338/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants