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

Additional Accessibilty enhancements #796

Closed

Conversation

nschonni
Copy link
Collaborator

Related to gh-706
Fixes gh-230

  • ARIA-Invalid works and is cleared properly on the resetForm() call
  • Add aria-required tags to any of the data, static or class required elements.

@jzaefferer I'm not sure where I should be hooking in to have rules defined in code be hooked in before the first validation call. Ex:

    form.validate({
        rules: {
            ariaRequiredDynamic: "required"
        }
    });

@@ -116,11 +113,11 @@ grunt.initConfig({
},
src: {
files: '<%= jshint.files %>',
tasks: ['concat']
tasks: ['concat', 'qunit']
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've changed this since I primarly run tests (and demos for manual testing) in the browser, and I want the concat task to run asap.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can revert that, but I found it useful for the immediate feedback from the watch task as I was developing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can see it being useful when you have more of a CLI focus while developing.

Maybe we can have multiple watch targets? For example, I could run "grunt watch:concat" to only get the concatenation?

@jzaefferer
Copy link
Collaborator

Thanks for this. It's great to finally have some progress in this area. I've left a few comments above.

@jzaefferer
Copy link
Collaborator

This seems to have been lost in another comment, I wrote: "The problem is then any elements that are added later, which the plugin supports thanks to event delegation. Back when it didn't have that, there was a refresh method. I hope we can avoid having to add that back..."

Any ideas how to address that?

As per http://www.w3.org/TR/wai-aria/states_and_properties#aria-invalid
the validation states should only be added after a user has had a chance
to enter a value
Added new test suite for testing the ARIA-invalid features
Also added a few minor fixes for typos in the QUnit scaffolding and
JSHint errors
Fix "%o has no name assigned [object HTMLInputElement]" errors in bypassValidation tests
Add aria-required tags to any of the data, static or class required elements.
Dynamic validation for rules added through the form.validate rules object do not get picked up till first run.
@nschonni
Copy link
Collaborator Author

I've fixed the commenting and quoting lines.


This seems to have been lost in another comment, I wrote: "The problem is then any elements that are added later, which the plugin supports thanks to event delegation. Back when it didn't have that, there was a refresh method. I hope we can avoid having to add that back..."

Any ideas how to address that?

Nope, that one stumped me. I tried plugging in the aria-required, but I couldn't find the correct spot to leverage the rules defined in settings. Reading http://www.w3.org/TR/WCAG-TECHS/ARIA2.html again, I think that flagging them required after the fact is still "OK", just less than ideal.
/cc @pjackson28 @LaurentGoderre

@nschonni
Copy link
Collaborator Author

nschonni commented Sep 4, 2013

@jzaefferer should this get landed now and address the dynamic element issues later?

@fagerli
Copy link

fagerli commented Oct 7, 2013

@jzaefferer Any chance of the aria-invalid part being included any time soon? Would be much appreciated!

@jzaefferer
Copy link
Collaborator

I've landed this in master, after a rebase. Unit tests look good in Chrome. After the rebase, the same 14 tests failed in IE8 as on master, so that should be fine as well. I'm not too happy about including something that isn't quite done, yet, but since I can't spend any time on this plugin these days, I hope its better to land as-is.

Thanks again @nschonni for your contributions. I appreciate those a lot!

@jzaefferer jzaefferer closed this Oct 7, 2013
@nschonni nschonni deleted the aria-required-support branch October 7, 2013 15:59
@nschonni
Copy link
Collaborator Author

nschonni commented Oct 7, 2013

No problem @jzaefferer I still owe you a Skype 😄

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.

Feature request: aria-required="true" when a field is required
4 participants