-
Notifications
You must be signed in to change notification settings - Fork 641
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
[a11y] Add popovers to verification checkmarks #8521
Conversation
f5d6500
to
57064af
Compare
|
||
for (var i = 0; i < listItemCount; i++) { | ||
var id = "reserved-indicator-" + i; | ||
configureCheckmarkImagePopover(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing this here, we should leverage the jQuery selectors we have available. Something like this:
$(".reserved-indicator").each(
function() {
var checkmarkImage = $(this);
checkmarkImage.popover({ trigger: 'hover focus' });
checkmarkImage.click(function() {
checkmarkImage.popover('show');
setTimeout(function() {
checkmarkImage.popover('destroy');
},
1000);
});
})
This should remove the need to pass listItemCount as a global var and should also remove the need to set an id on each element.
The above should be able to replace the for loop and the configureCheckmarkImagePopover function below and I believe is functionally identical.
@@ -244,5 +244,9 @@ | |||
$('#reset-advanced-search').on('click', function () { | |||
location.href = '[email protected](Model.SearchTerm)'; | |||
}); | |||
|
|||
var listItemCount = @Model.Items.Count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this if we use jQuery class selectors instead. See other comment.
@@ -50,7 +50,8 @@ | |||
<img class="reserved-indicator" | |||
src="~/Content/gallery/img/reserved-indicator.svg" | |||
@ViewHelpers.ImageFallback(Url.Absolute("~/Content/gallery/img/reserved-indicator-20x20.png")) | |||
title="@Strings.ReservedNamespace_ReservedIndicatorTooltip" /> | |||
data-content="@Strings.ReservedNamespace_ReservedIndicatorTooltip" | |||
id=@("reserved-indicator-" + ViewData["ItemIndex"]) tabindex="0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id does not need to be set here if you use the class to enumerate the images that need the popover configured. See above.
Addresses: #8490
We need a tab stop on this checkmark (
tabindex='0'
means it won't mess with tab ordering), and an id that can be found by the popover code. This will respond to hover and focus events.Screenreaders also read the text as written (tested on Narrator and NVDA).
Just realized how fast this popover flashes by in the below video. The text is the same as the current tooltip:
The ID prefix of this package has been reserved for one of the owners of this package by NuGet.org