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

Textual facets in collapsible on screen #274

Merged
merged 13 commits into from
Nov 12, 2024
Merged

Conversation

jrochkind
Copy link
Member

@jrochkind jrochkind commented Nov 12, 2024

A "universal design" approach to the need for textual facets corresponding to the chart for low-vision or screen-reader-using users.

Putting all the links in a visually-hidden didn't work, becuase you can't actually put focasable things in visually-hidden without problems or more weirdness, so trying this.

Put them on all screens, but inside a collapsible header, to avoid noise for those who don't want them. Went with html5 <details> after all, does not seem to have any focus nav issues to me.

Rearranged some DOM and configuration and made some selectors more flexible in the process.

Still needs i18n on label, and some other cleanup.

There is a lot of info and controls in this panel, it was hard to make it all hopefully clear?


Screenshot 2024-11-12 at 10 55 17 AM


Screenshot 2024-11-12 at 10 55 36 AM


@seanaery
Copy link
Collaborator

That looks great to me, @jrochkind . Bonus points for making it configurable. I like the way the details & summary tags work here, and it's nicely responsive and keyboard navigable. As you noted, an i18n key for the label is needed so implementers have an easy hook to customize that. Here are screenshots running this branch in my demo app.

Screenshot 2024-11-12 at 2 20 01 PM Screenshot 2024-11-12 at 2 20 09 PM

@jrochkind
Copy link
Member Author

OK, i18n added, initial non-English translations provided by Google Translate via i18n-tasks.

@jrochkind jrochkind marked this pull request as ready for review November 12, 2024 21:13
@jrochkind
Copy link
Member Author

@seanaery What do you think about that heading "Range List"?

I'm not sure anyone will have any idea what it means... but having trouble coming up with anything.

Colleague @eddierubeiz suggests "Number of results in each range", or "Results in each range", or maybe "Results in each interval"...

I really don't know.

@seanaery
Copy link
Collaborator

Right, the default label is tricky. I do like Range List on-screen because it's concise; I might slightly prefer List Ranges given how it uses "list" as a verb. I hesitate to use more than 2-3 words there -- definitely don't want it to wrap, and we have to accommodate all kinds of BL layouts, including ones that don't leave a lot of width for the facets.

I assume esp. with the details/summary tags that the aim is to have one label that doesn't change to reflect the open/closed state. If there was an easy way to toggle, maybe Show Ranges / Hide Ranges works, but not sure that's worth the added complexity.

@jrochkind
Copy link
Member Author

Yeah, I think it would have to add custom JS to switch label for open/closed, which I was trying to avoid.

Want to approve and merge what's here now?

@seanaery seanaery self-requested a review November 12, 2024 21:32
@seanaery seanaery merged commit 7820e67 into main Nov 12, 2024
9 checks passed
@seanaery seanaery deleted the collapsible_textual_facets branch November 12, 2024 21:33
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.

2 participants