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

Consolidate input aria documentation and tests #1062

Merged
merged 1 commit into from
Aug 23, 2019

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Aug 21, 2019

This PR has been updated and no longer does what it originally did.

What

Consolidate the aria attribute documentation for the input component.

A possible future change (for more than just this component) could be to pass any required aria attributes to components rather than having to add new options for each new use case, as below.

<%= render "govuk_publishing_components/components/input", {
  label: {
    text: "This is an example only and may not work with a screen reader"
  },
  name: "labelledby",
  aria: {
    describedby: "wrapper",
    controls: "wrapper"
  }
} %>
aria ||= []
<%= tag.input name: name,
    aria: aria
%>

Visual Changes

No visual changes.

View Changes

https://govuk-publishing-compo-pr-1062.herokuapp.com/component-guide/input

@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1062 August 21, 2019 08:52 Inactive
@NickColley
Copy link
Contributor

Could we avoid doing this by updating the pattern on that page to match the current recommendations for errors? I don't believe we promote using aria-invalid at the moment, so might be able to simplify things this way.

@NickColley
Copy link
Contributor

I've made an example of this: https://govuk-frontend-local-council.glitch.me/

@NickColley
Copy link
Contributor

@36degrees did an investigation here: alphagov/govuk-frontend#1168

@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1062 August 21, 2019 16:16 Inactive
@andysellick
Copy link
Contributor Author

@NickColley you're absolutely right. I was so busy trying to implement this feature I didn't stop to consider if we actually needed it or not. I've reworked this PR to not have this addition but still have the consolidation of the aria examples and tests, which I think is worth keeping. Thanks!

@andysellick andysellick changed the title Add aria-invalid to input component Consolidate input aria documentation and tests Aug 21, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1062 August 21, 2019 16:21 Inactive
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.

4 participants