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

[EuiProgress] Add semantic structure to label and value #3678

Closed
myasonik opened this issue Jul 1, 2020 · 4 comments
Closed

[EuiProgress] Add semantic structure to label and value #3678

myasonik opened this issue Jul 1, 2020 · 4 comments

Comments

@myasonik
Copy link
Contributor

myasonik commented Jul 1, 2020

Pulling out an idea from PR #3218 into a follow up issue.

Trying to think of a way to join the label and value text so there's something more semantic tying them together rather than just proximity...

I'm thinking maybe a <dl> might work? A DOM structure like this maybe:

<dl>
	<dt>Men's clothing</dt>
	<dd>80%</dd>
	<dd aria-hidden="true"><progress /></dd>
</dl>

If we introduced an extra prop for how to render the wrapping element, we could even extend this to better work with a whole list of these like in the demo. Something like:

<dl>
	<EuiProgress as="div" value={20} label={"Men's clothing"} max={100} />
	<EuiProgress as="div" value={20} label={"Women's clothing"} max={100} />
	<EuiProgress as="div" value={20} label={"Women's shoes"} max={100} />
	<EuiProgress as="div" value={20} label={"Men's shoes"} max={100} />
</dl>

Where as changes the wrapping <dl> of each <EuiProgress /> to a <div> so that we can create one list of progress bars, instead of n lists. (Maybe as is too vague and broad given that <div> is probably the only valid value, but just throwing out an idea.)

@myasonik
Copy link
Contributor Author

myasonik commented Jul 1, 2020

Comments from @cchaos:

I definitely see the benefit of using the <dl> type of structure but it seems way to prescriptive to do this for every single usage.
- What happens if it's just a single progress bar? <dd aria-hidden> doesn't work by itself.
- What if it's only a single use of the progress with a label? Is it ok to have a list with a single item?

Let's consider just making the elements customizable, or we'll need to create a higher component like EuiListGroup that will create the whole element structure.

With the risk of this PR continually expanding greater, let's table this discussion and put it into an issue for follow up.

@myasonik
Copy link
Contributor Author

myasonik commented Jul 1, 2020

What happens if it's just a single progress bar? <dd aria-hidden> doesn't work by itself.

Ah, yeah, you're right. I guess we only go to the <dl> structure if there's a value and label then.

What if it's only a single use of the progress with a label? Is it ok to have a list with a single item?

Yup, should be fine!

@cchaos cchaos changed the title [EuiProgress] add semantic structure to label and value [EuiProgress] Add semantic structure to label and value Sep 20, 2020
@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@daveyholler
Copy link
Contributor

Not planned as the current implementation is acceptable.

@daveyholler daveyholler closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants