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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiHorizontalStep] Added current status #4911

Merged
merged 9 commits into from
Jul 27, 2021

Conversation

snide
Copy link
Contributor

@snide snide commented Jun 23, 2021

Summary

credit @cnasikas for the spot.

Hopefully I still remember how to code 馃槃

@cchaos has taken over 馃槒

Default state of horizontal step should be incomplete not undefined. This aligns it with the proper usage which differs from our vertical steps since we want to always denote progress. The docs were also fixed from using deprecated props, to the correct status.

Which also meant adding current as a possible status option.

Default theme

Screen Shot 2021-07-26 at 14 18 14 PM

Step 3 looks "broken" in this screenshot, but it's technically correct since there should only be one current/selected step, I'm just showing how using either prop still works.

Amsterdam

Screen Shot 2021-07-26 at 14 11 06 PM

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 and playground toggles
  • 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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@snide
Copy link
Contributor Author

snide commented Jun 23, 2021

In talking with @cchaos, we think it makes sense to add current as a status, rather than just leaving it to the default state. Will update this PR when I have time later today.

cchaos added 3 commits July 26, 2021 11:38
*Breaking* Makes EuiHorizontalStep current default to `incomplete`
@kibanamachine
Copy link

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

@cchaos cchaos changed the title Fix default status of horizontal steps [EuiHorizontalStep] Fixed default status Jul 26, 2021
@cchaos cchaos changed the title [EuiHorizontalStep] Fixed default status [EuiHorizontalStep] Added current status Jul 26, 2021
cchaos added 2 commits July 26, 2021 15:12
@kibanamachine
Copy link

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

@cchaos
Copy link
Contributor

cchaos commented Jul 26, 2021

Ok, this one is ready for review again 馃槈 . The summary was also updated.

Copy link
Contributor Author

@snide snide left a comment

Choose a reason for hiding this comment

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

I can't approve this because I made it (kinda), but I approve @cchaos's changes. Thanks for picking it up and finishing it off.

@myasonik myasonik self-requested a review July 27, 2021 15:58
Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

Code and docs lgtm 馃殌

@kibanamachine
Copy link

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

@cchaos cchaos merged commit de5873f into elastic:master Jul 27, 2021
cchaos pushed a commit to myasonik/eui that referenced this pull request Jul 27, 2021
Co-authored-by: cchaos <caroline.horn@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants