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

STCOM-1192 Group checkboxes in FilterGroups (a11y) #2226

Merged
merged 4 commits into from
Feb 28, 2024
Merged

Conversation

JohnC-80
Copy link
Contributor

@JohnC-80 JohnC-80 commented Feb 16, 2024

https://folio-org.atlassian.net/browse/STCOM-1192

Related inputs are commonly group within a <fieldset> element or a div element using role=group. Both do similar things, assigning a top-level label. The downside to fieldset is that it has a built-in styling that has to be overridden and <legend> elements can also be stubborn for styling, although that is less of a matter. A div[role="group"] with an aria-label suffices.

<Filtergroups> often wouldn't be able to render a decent unique id due to objects (<FormattedMessage>) being passed as the label to the groups, despite propTypes dictating it be a string. A small utility function was used to derive the string from a passed FormatMessage instance, converting it to a call to formatMessage fron Intl contenxt.

Proptypes of FilterGroups were expanded to relax common proptype errors in the console and we reduce the number of id="[object Object]" occurrences in the DOM.

image

From Snapshot Environment: [object Object] id's... egad!
image

Ah... no longer (after change)
image

Copy link

github-actions bot commented Feb 16, 2024

Jest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit a54f19e. ± Comparison against base commit 7d979e1.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Feb 16, 2024

BigTest Unit Test Statistics

       1 files  ±0         1 suites  ±0   13s ⏱️ ±0s
1 420 tests ±0  1 414 ✔️ ±0  6 💤 ±0  0 ±0 
1 423 runs  ±0  1 417 ✔️ ±0  6 💤 ±0  0 ±0 

Results for commit a54f19e. ± Comparison against base commit 7d979e1.

♻️ This comment has been updated with latest results.

@@ -314,7 +327,7 @@ FilterGroups.propTypes = {
config: PropTypes.arrayOf(
PropTypes.shape({
cql: PropTypes.string.isRequired,
label: PropTypes.string.isRequired,
label: PropTypes.oneOfType([PropTypes.string, PropTypes.element]).isRequired,
Copy link
Member

Choose a reason for hiding this comment

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

made a stripes-types edit folio-org/stripes-types#55

Copy link

@JohnC-80 JohnC-80 merged commit 85ebbbb into master Feb 28, 2024
6 checks passed
@JohnC-80 JohnC-80 deleted the STCOM-1192 branch February 28, 2024 18:07
github-actions bot pushed a commit that referenced this pull request Feb 28, 2024
* accept full label id from outside of the component via headerProps

* allow for elements in label proptype. Derive string from within FormatMessage, if supplied.

* Get string from possible FormattedMessage in group label for Accordion id of FilterGroup

* log changes
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