Fix accessibility issues identified by tool #5795
Conversation
scottbommarito
requested review from
joelverhagen,
agr,
loic-sharma,
xavierdecoster,
shishirx34,
nugetlurker,
ryuyu,
chenriksson,
cristinamanum and
skofman1
| showErrors: function (errorMap, errorList) { | ||
| this.defaultShowErrors(); | ||
|
|
||
| var i; |
There was a problem hiding this comment.
JQuery Validate adds an aria-describedby attribute to every validated field, even if the validation is successful. This is wrong because if there is no error, the aria-describedby leads to an empty element, which is invalid. This code removes the unnecessary attribute.
EDIT: going to put this in a comment for future's sake
| .navbar-logo { | ||
| margin: 8px 20px 0 0; | ||
| } | ||
| .navbar-seperated .seperator { | ||
| .navbar-separated .seperator { |
There was a problem hiding this comment.
Fix the seperator as well, maybe?
There was a problem hiding this comment.
apparently we don't even have a seperator, which means the entire navbar-separated class is useless
removed it entirely
| @@ -50,10 +50,48 @@ | |||
| } else { | |||
| error.insertAfter(element); | |||
| } | |||
| }, | |||
| showErrors: function (errorMap, errorList) { | |||
There was a problem hiding this comment.
jquery.validate calls it whenever the state of our forms has changed. It adds validation error text to our forms when the content isn't correct. I'm using their default process and appending an additional step.
| }); | ||
|
|
||
| if (ids.length) { | ||
| element.setAttribute("aria-describedby", ids.join(" ")); |
There was a problem hiding this comment.
Only call `setAttribute if the IDs set has changed.
There was a problem hiding this comment.
Is setAttribute an expensive operation? I feel like the code is much simpler like this.
| @@ -44,9 +44,9 @@ | |||
| </div> | |||
| </div> | |||
| <div class="row text-center create-provider-account"> | |||
There was a problem hiding this comment.
There should only be one of these... not one per provider. You can put it in a Model.Providers.Count > 0 if depending on what @shishirx34 things.
https://github.com/NuGet/Engineering/issues/1359
This PR fixes a number of small issues with the accessibility of our site as identified by a tool.
Mostly consists of changing roles and adding labels.