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

[Steps] Small-sized horizontal steps #6850

Closed
7 tasks done
JasonStoltz opened this issue Jun 20, 2023 · 12 comments · Fixed by #6928
Closed
7 tasks done

[Steps] Small-sized horizontal steps #6850

JasonStoltz opened this issue Jun 20, 2023 · 12 comments · Fixed by #6928
Assignees

Comments

@JasonStoltz
Copy link
Member

JasonStoltz commented Jun 20, 2023

We would like to add small-sized variant of the EuiStepsHorizontal component.

Description

This was originally proposed in this discussion.

A design spec was created to support this.

There is also a Figma design for this.

Tasks

Acceptance criteria

@JasonStoltz
Copy link
Member Author

JasonStoltz commented Jun 20, 2023

@1Copenut @ryankeairns I assigned this to both of you for the tasks above.

Ryan, would you mind reviewing the spec and helping us reconcile the Figma and spec?

@1Copenut do you think there are any a11y implications to consider when not showing a visible step number?

@1Copenut
Copy link
Contributor

1Copenut commented Jun 22, 2023

@JasonStoltz Yes, I do think there's a11y implications. I'm reviewing the component now, and will update this comment if I find additional things. Here's what I feel worth looking into:

  1. The current (blue) dot is very similar in hue and intensity to the completed (green) steps. The completed steps do have a black check mark, but at small size I'm not sure this is enough to meet the requirements of SC 1.4.1: Use of Color (Level A). Testers might argue the checkmark is too small to be seen by users with reduced visual acuity. I'd recommend adding a differentiation to the current step, like a bold outer halo of the same color with 2-4px of whitespace between the dot and the outer border. We could also add the word "current" or something to the label to indicate this is the current item.
  2. The disabled text color #ABB4C4 does not meet WCAG 4.5:1 contrast ratio. I'd recommend making this text the same color as non-current labels and spelling out that the step is disabled.
  3. Safari + VO is reading out a lot of extra information in our live horizontal list. I think it's the combined title and svg[aria-label] being picked up. This might not affect NVDA / JAWS. I'll test it after bit and add a code improvement ticket if necessary.

@ryankeairns
Copy link
Contributor

I believe we can scope this down to adding a size prop to the horizontal layout. It would support two values: m as default (which is the current design) and s which is a new option. In this option, the only thing that changes is the size of the circular marker; the title size is unaffected.

There is an existing use case for s during the create deployment/project flow, but I am not aware of anything for xs which at this size or lower could run us into avoidable a11y issues.

cc:/ @cindychangy

@JasonStoltz
Copy link
Member Author

JasonStoltz commented Jun 23, 2023

Just so I'm clear. We're proposing we use this size, from the Figma -- and not the smallest size.

Screen Shot 2023-06-23 at 11 53 36 AM

In which case, we're ditching the idea of the "numberless" format and "xs" size. We would still have numbers at the "s" size.

Please note that if this is the case, we should update the Figma and spec as well, so there's zero confusion when we get to work on this.

@JasonStoltz
Copy link
Member Author

JasonStoltz commented Jun 26, 2023

@ryankeairns Can I get your input on the above question?

@ryankeairns
Copy link
Contributor

That is correct. Add small but not smallest.

@JasonStoltz JasonStoltz assigned breehall and unassigned 1Copenut Jun 26, 2023
@JasonStoltz
Copy link
Member Author

@cindychangy Hi Cindy 👋 Could you please update the horizontal step variant Figma to reflect the changes that Ryan suggested?

  1. Remove all references in that file that use the smallest size (the one with numberless format), and replace them with the "small" size icons.
small

@cindychangy
Copy link

cindychangy commented Jul 3, 2023

@JasonStoltz - noted and will remove that reference in Figma, thanks!

@breehall
Copy link
Contributor

breehall commented Jul 5, 2023

@1Copenut

The completed steps do have a black check mark, but at small size I'm not sure this is enough to meet the requirements of SC 1.4.1: Use of Color (Level A).

Would the image below be an example of what you're proposing for steps that are complete?
image

We could also add the word "current" or something to the label to indicate this is the current item.

Would this be with aria? Currently, there's an aria-label map set up to provide the status of each step. For example, when a step is complete, the aria-label will read Step 1 is complete. Is there another attribute we need to use to alert the user of the step status?

@1Copenut
Copy link
Contributor

@breehall
This outer halo is awesome. I think it'd be more effective on the current step, because there'd only ever be one and it's a subtle visual distinction.

If completed steps have aria that announces a clear response, I think that and the checkbox should be good overlapping signals this step is complete.

For the current step, I think we can follow suit with aria that announces "Step two is the current step" or something similar and the outer halo that visually distinguishes it from the others.

Feel free to ping me in the morning if you'd like to discuss this more!

@breehall
Copy link
Contributor

breehall commented Jul 11, 2023

Thank you for the clarification Trevor! Since this would be a change to the design of EuiSteps both horizontal and vertical, I wanted to loop in @andreadelrio as well.

Just so everything is in one place, the design changes being proposed are:

  1. Adding a halo around steps that have the current status (like the image below)
    image

  2. Boosting the contrast ratio of the disabled step text

Both of these items affect both versions of EuiSteps, so I'm proposing that I create a secondary PR to handle these outside of the new prop. I also found a slight bug in the alignment of status icons for the vertical version of EuiSteps (below), so we could loop the alignment fix into the new PR as well.

Edit: It seems that this issue only happens in CodeSandbox. I'm trying to see if a style isn't loading / what's going on, but this doesn't look like an EUI bug.

image

What do you all think?

@ryankeairns
Copy link
Contributor

+1 for a follow-up, separate PR

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