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

Fix: Make input prefix/suffix params mandatory to render component #2349

Merged
merged 8 commits into from
Aug 1, 2022

Conversation

jrbarnes9
Copy link
Contributor

@jrbarnes9 jrbarnes9 commented Jul 21, 2022

What is the context of this PR?

Fixes #2344

  • Added conditions to input macro to ensure suffix/prefix objects have id set because it's required for aria-labelledby on the input
  • Added conditions to input macro to ensure suffix/prefix objects have title set because it's required for <abbr> element to be valid
  • Added new macro tests for these conditions
  • Updated macro tests and examples to add these required params

What are the breaking changes?

  • All inputs using params.prefix must set both params.prefix.id and params.prefix.title to render.
  • All inputs using params.suffix must set both params.suffix.id and params.suffix.title to render.

How to review

  • Check tests pass and that all prefix/suffix inputs cannot render unless title and id are set.

@jrbarnes9 jrbarnes9 changed the title 2344 suffix prefix empty aria Fix: Make input prefix/suffix params mandatory to render component Jul 21, 2022
@jrbarnes9 jrbarnes9 self-assigned this Jul 21, 2022
@jrbarnes9 jrbarnes9 added the Bug Something isn't working label Jul 21, 2022
@jrbarnes9 jrbarnes9 marked this pull request as ready for review July 21, 2022 15:09
@jrbarnes9 jrbarnes9 requested review from rmccar, kruncher and boxadesign and removed request for rmccar and kruncher July 21, 2022 15:09
@rmccar rmccar added the Breaking changes This PR contains at least one breaking change label Jul 21, 2022
kruncher
kruncher previously approved these changes Jul 21, 2022
Copy link
Contributor

@kruncher kruncher left a comment

Choose a reason for hiding this comment

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

Looks good to me

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
@jrbarnes9 jrbarnes9 merged commit 574137b into main Aug 1, 2022
@jrbarnes9 jrbarnes9 deleted the 2344-suffix-prefix-empty-aria branch August 1, 2022 09:55
boxadesign pushed a commit that referenced this pull request Feb 17, 2023
…2349)

* added conditions to require prefix/suffix params

* updated macro options for durations suffix to document objects params

* updated all prefix/suffix examples to make sure they have all required params [test-visual]

* updated mutually exclusive tests to add mandatory prefix/suffix id

* added condition to require prefix/suffix title to render

* added input tests to check for prefix/suffix id [test-visual]

* Update src/components/input/_macro.njk

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking changes This PR contains at least one breaking change Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suffix/prefix aria-labelledby is required but can be left empty
3 participants