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

Web Docs - Stepper Component #2727

Open
wants to merge 43 commits into
base: dchyun/stepper-component
Choose a base branch
from

Conversation

majedelass
Copy link
Contributor

@majedelass majedelass commented Feb 27, 2025

πŸ“Œ Summary

If merged, this PR will add the Stepper component documentation to the website.

πŸ› οΈ Detailed description

The Stepper documentation is getting slightly restructured, where the previous Stepper Indicator will now live under a Stepper category, along side Stepper Navigation and Stepper List.

πŸ“Έ Screenshots

Before
Screenshot 2025-02-27 at 11 01 24β€―AM

After
Screenshot 2025-03-03 at 12 03 04β€―PM

πŸ”— External links

Jira ticket: HDS-4405


πŸ’¬ Please consider using conventional comments when reviewing this PR.

@majedelass majedelass requested review from a team as code owners February 27, 2025 02:26
Copy link

vercel bot commented Feb 27, 2025

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
hds-showcase βœ… Ready (Inspect) Visit Preview Mar 12, 2025 7:31am
hds-website βœ… Ready (Inspect) Visit Preview Mar 12, 2025 7:31am

Copy link
Contributor

@heatherlarsen heatherlarsen left a comment

Choose a reason for hiding this comment

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

It's a good start. I think the separation between the two pages also helps make it more clear that these have two distinct use cases. I left quite a few suggestions and a few comments, but nothing too major.

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

I haven't gotten through all of this yet, but I'll take a stab at the Nav variant tomorrow.

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

Ok I got through the rest of this now, dropped some more notes and some non-blocking grammar suggestions. LMK if there's anything that needs clarification!


Focus on the active step.

![Example Stepper Nav with 3 steps showing on tab press the second step, which is active, is focused](/assets/components/stepper/navigation/stepper-navigation-focus.png =1400x*)
Copy link
Contributor

Choose a reason for hiding this comment

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

[question] Are these screenshots from the Showcase? I'm questioning whether why the width is explicitly set to 1400px, which I don't think they will ever render at. Instead, should those screenshots be dropped into (or recreated in ) Figma and resized? For full-bleed images we generally size images at 836px (I know, also somewhat arbitrary), but that largely removes the need to set the width in markdown explicitly

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this one is on me. I was following what was done for the tabs accessibility docs and there they set what seemed an arbitrary width of 402px on the images. I also picked 1400 arbitrarily. I removed explicitly setting the width for now.

I made these examples locally to show the interactions. I'm not sure what the standard way we make these kind of examples is, but if it's preferable to create them in Figma instead we can do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, It's somewhat of an ambiguous "process", happy to pair with you on it if that would help, but @majedelass is also privy on the dimensions

Copy link
Contributor

Choose a reason for hiding this comment

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

@majedelass I removed explicitly setting the width on the images. Do you think they need any other updates? Or do you want to recreate them in figma and change the example used?

Copy link
Contributor

@KristinLBradley KristinLBradley left a comment

Choose a reason for hiding this comment

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

@majedelass @dchyun I think we are getting close but could you both please review the current suggested edits and apply them if you agree? (Or else tweak them?)

Then I think we can wrap this up.


## Interactive vs non-interactive

The Stepper Nav supports both interactive (default) and non-interactive variants.
Copy link
Contributor

Choose a reason for hiding this comment

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

[Issue] We may need to discuss this, but currently the nav is non-interactive by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is currently based off of the requirements doc in the variants section indicating that isInteractive set to true is the default.

Do we believe that our consumers will use the non-interactive variant more often than the alternative? That's usually how we define what is set by default.

| Title | Required |
| Description | Optional |
| Body | Optional; accepts any custom component instance (local or from Helios)|
| Progress bar | Required |
Copy link
Contributor

Choose a reason for hiding this comment

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

[Note] Not sure if this is worth documenting, but just fyi if there is only one step the progress bar will not be shown.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there's only one step, the Stepper shouldn't be used but maybe we could include an explanation in parenthesis in case that needs to be clarified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the when to use section says "multi-step flow" meaning that it should never be 1 step only.

| Title | Required |
| Description | Optional |
| Body | Optional; accepts any custom component instance (local or from Helios)|
| Progress bar | Required |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| Progress bar | Required |
| Progress bar | Required (not shown if only one step) |

Copy link
Contributor Author

@majedelass majedelass Mar 12, 2025

Choose a reason for hiding this comment

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

I think we should not have this in our docs. We do not recommend folks to only have one step.

</Hds::Stepper::Nav>
```

### Without panels
Copy link
Contributor

Choose a reason for hiding this comment

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

@shleewhite I added a section for using the nav without panels, which we've discussed earlier. Please let me know if the accessibility notes here look correct, or if any changes need to be made to the accessibility file as well.

Co-authored-by: Dylan Hyun <[email protected]>
Co-authored-by: Kristin Bradley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-website Content updates to the documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants