Fix (most) accessibility issues found by recent scan #6215
Conversation
scottbommarito
requested review from
joelverhagen,
agr,
loic-sharma,
xavierdecoster,
shishirx34,
ryuyu,
chenriksson,
cristinamanum and
skofman1
| @@ -10,7 +10,8 @@ | |||
| "[email protected]" | |||
| ], | |||
| "ForceTransformationToOrganizationForEmailAddresses": [ | |||
| "[email protected]" | |||
| "[email protected]", | |||
| "[email protected]" | |||
| ], | |||
| "EnabledOrganizationAadTenants": [ | |||
| { | |||
There was a problem hiding this comment.
noooo.. don't commit these changes.
There was a problem hiding this comment.
lol yeah i fixed this like <5 min later
| @@ -10,6 +10,8 @@ | |||
| _addCertificateUrl = addCertificateUrl; | |||
| _getCertificatesUrl = getCertificatesUrl; | |||
|
|
|||
| window.nuget.configureFileInputButton("register-new"); | |||
There was a problem hiding this comment.
Maybe useful to add accessibility comments - so we don't accidentally revert? Would also help w/ review(s).
// Accessibility: register new certificate button should respond to keyboard events
window.nuget.configureFileInputButton("register-new");
| }); | ||
| } | ||
|
|
||
| configureSection(sections[i]); |
There was a problem hiding this comment.
So only ManageOrganizations view had working cancel buttons? This extends it to others (SignIn, Account, Packages)?
There was a problem hiding this comment.
Most cancel buttons on the site worked. This was specifically an issue between Manage Organizations and Account Settings. When those pages were de-duped we must have accidentally forgot that the script that configures the cancel buttons needed to be placed on both individually.
| (isElement("input") && element.attr("type") !== "hidden") || | ||
| (isElement("menu") && element.attr("type") !== "toolbar") || | ||
| (isElement("object") && hasAttribute("usemap")) || | ||
| (isElement("video") && hasAttribute("controls"))); |
There was a problem hiding this comment.
Maybe isElement should be defined off nuget too?
Could also introduce nuget.isTabbableElement(e) for the logic in this return statement, and possibly `nuget.IsAlwaysInteractiveElement(e) for the above logic.
There was a problem hiding this comment.
isElement captures the element that was passed into canElementBeTabbedTo so it can't be on nuget (same with hasAttribute)
I want to avoid adding too many unnecessary functions to nuget. To be honest, I wasn't going to add canElementBeTabbedTo or getFirstChildThatCanBeTabbedTo but I thought the code looked much cleaner with them defined outside of the $(#skipToContent) click handler code. In the future if we determine we need to use parts of this implementation elsewhere, we can quickly make the change to split these methods then.
| } | ||
|
|
||
| // See https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Interactive_content | ||
| var alwaysInteractiveElements = ['a', 'button', 'details', 'embed', 'iframe', 'keygen', 'label', 'select', 'textarea']; |
There was a problem hiding this comment.
Should this list be defined as a constant off nuget?
There was a problem hiding this comment.
Same as above...I don't think it's super useful to have it available. If we need it somewhere else we can move it to nuget then.
|
After discussing with @anangaur - remove "delete organization" link for organization owners, keep delete icon for admins |
|
Waiting on #6266 to merge |
#6152
This PR fixes the following issues:
635011 - "Skip to Content" link loses tab order
My solution here was to write a script that sets the focus on first element in the content that can be tabbed to.
636482 - Discontinued login modal dialog does not set focus correctly
636486 - Register new certificate button does not respond to keyboard events
638489 - Recaptcha badge in contact forms tab order is incorrect
(note: this currently does not fix the issue due to a bug in Recaptcha)
638490 - Browse for readme button does not respond to keyboard events
643610 / 644048 - Links that are styled like buttons are displaying like links in high contrast mode
643613 - Profile page "delete user" link tab order is incorrect
My solution here was to change the "delete user" text into an icon alongside the "manage account" breadcrumb.
643615 - Email notifications form label name is incorrect
643617 - Profile page "manage my organizations" breadcrumb should read the number of organizations you own
644030 - Owner dropdown in upload form should read correctly in screenreader
644070 - Number of packages returned in search should be announced by screenreader
Functional - "Cancel" buttons do not work on the Account Settings page.