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

Accessible Name for URL Input #527

Merged
merged 8 commits into from
Aug 15, 2018
Merged

Conversation

dctalbot
Copy link
Contributor

Found this on a code sniffer.

This url input element does not have a name available to an accessibility API.
Valid names are:

  • title
  • aria-label
  • aria-labelledby
  • label element

If you would like this to go forward, please let me know which attribute you would like to use and its value.

Reference:
Success Criterion

@javan
Copy link
Contributor

javan commented Aug 12, 2018

/cc @bergatron

@bergatron
Copy link
Member

Good catch @dctalbot!

@@ -29,7 +29,7 @@ Trix.config.toolbar =
<div class="trix-dialogs" data-trix-dialogs>
<div class="trix-dialog trix-dialog--link" data-trix-dialog="href" data-trix-dialog-attribute="href">
<div class="trix-dialog__link-fields">
<input type="url" name="href" class="trix-input trix-input--dialog" placeholder="#{lang.urlPlaceholder}" required data-trix-input>
<input type="url" name="href" class="trix-input trix-input--dialog" placeholder="#{lang.urlPlaceholder}" aria-label="#{lang.urlAccessibility}" required data-trix-input>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the aria-label should just be "URL". You wouldn't use "Enter a URL" or "Enter a name" as a visible <label>s for form inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I pushed a commit to reflect this change.

@@ -17,6 +17,7 @@ Trix.config.lang =
strike: "Strikethrough"
undo: "Undo"
unlink: "Unlink"
urlAccessibility: "URL"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's drop Accessibility from this key. Just url is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@javan javan merged commit 1f9b090 into basecamp:master Aug 15, 2018
@javan
Copy link
Contributor

javan commented Aug 15, 2018

Thank you!

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.

3 participants