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.

Sign up for GitHub

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

Jump to bottom

[a11y] Add popovers to verification checkmarks #8521

Merged
merged 2 commits into from Apr 16, 2021
Merged

[a11y] Add popovers to verification checkmarks #8521

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57064af
Add popovers to verification checkmarks
drewgillies Apr 12, 2021
48c6ff3
Addressed feedback
drewgillies Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/NuGetGallery/App_Start/AppActivator.cs
View file
Open in desktop
Expand Up @@ -170,6 +170,10 @@ private static void BundlingPostStart()
.Include("~/Scripts/gallery/clamp.js");
BundleTable.Bundles.Add(displayPackageScriptBundle);

var listPackagesScriptBundle = new ScriptBundle("~/Scripts/gallery/page-list-packages.min.js")
.Include("~/Scripts/gallery/page-list-packages.js");
BundleTable.Bundles.Add(listPackagesScriptBundle);

var managePackagesScriptBundle = new ScriptBundle("~/Scripts/gallery/page-manage-packages.min.js")
.Include("~/Scripts/gallery/page-manage-packages.js");
BundleTable.Bundles.Add(managePackagesScriptBundle);
Expand Down
1 change: 1 addition & 0 deletions src/NuGetGallery/NuGetGallery.csproj
View file
Open in desktop
Expand Up @@ -1675,6 +1675,7 @@
<Content Include="App_Data\Files\Content\Symbols-Configuration.json" />
<Content Include="Scripts\gallery\instrumentation.js" />
<Content Include="Scripts\gallery\knockout-3.5.1.js" />
<Content Include="Scripts\gallery\page-list-packages.js" />
<Content Include="Views\Shared\SiteMenu.cshtml">
<SubType>Code</SubType>
</Content>
Expand Down
22 changes: 22 additions & 0 deletions src/NuGetGallery/Scripts/gallery/page-list-packages.js
View file
Open in desktop
@@ -0,0 +1,22 @@
$(function () {
'use strict';

for (var i = 0; i < listItemCount; i++) {
var id = "reserved-indicator-" + i;
configureCheckmarkImagePopover(id);
Copy link
Contributor

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.

}

function configureCheckmarkImagePopover(id) {
var checkmarkImage = $('#' + id);
if (checkmarkImage.length == 1) { // i.e. checkmark exists
checkmarkImage.popover({ trigger: 'hover focus' });
checkmarkImage.click(function() {
checkmarkImage.popover('show');
setTimeout(function() {
checkmarkImage.popover('destroy');
},
1000);
});
}
}
});
4 changes: 4 additions & 0 deletions src/NuGetGallery/Views/Shared/ListPackages.cshtml
View file
Open in desktop
Expand Up @@ -244,5 +244,9 @@
$('#reset-advanced-search').on('click', function () {
location.href = '?q=@Uri.EscapeDataString(Model.SearchTerm)';
});

var listItemCount = @Model.Items.Count;
Copy link
Contributor

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.

</script>

@Scripts.Render("~/Scripts/gallery/page-list-packages.min.js")
}
3 changes: 2 additions & 1 deletion src/NuGetGallery/Views/Shared/_ListPackage.cshtml
View file
Open in desktop
Expand Up @@ -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" />
Copy link
Contributor

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.

}

@if (showEditButton && (Model.CanEdit || Model.CanManageOwners || Model.CanUnlistOrRelist))
Expand Down