From 89c2c936c78f5b3933f94ff2898d86c33d19eac1 Mon Sep 17 00:00:00 2001 From: thewildmage Date: Mon, 31 Oct 2022 10:18:51 -0600 Subject: [PATCH] Improve navigation accessibility and usability (#544) * Navbar: Make more consistent, remove unnecessary nav element * Use updated navbar, move filters closer to filtered content * Rename "navbar" to "navigation" For improved clarity and to avoid tying design to naming * Fix indentation * Use hidden attribute instead of display none * Simplify for loops * Rearrange script * Use generic names for generic functions * Remove filter submit button Is wired up and working, but doesn't feel like it because results update without using it * Update filter HTML * Remove excess article element * Remove extra namespace list status It's been folded into the filter template * First pass at filter status updates * Remove listener for now non-existent button * Handle case where filter starts non-empty * Don't use input type=search * Fewer announcements, handle same announcements back to back * Add explanatory comments * Filter script: Consolidate editing outputs * Update form hide if no filterable entries found * Filter.js: Consolidate variables, update comments, test HTML presence * Convert filter.js to module, extract common utilities * Add main.js with setJavascriptAvailable function * Handle no JavaScript situation * Make filter context consistent across pages * Only add filter if more than 1 entry * Hide Glossary link when it's not on page * Update filter styling * Hide "Limit results" link if only namespace Co-Authored-By: Andrew Suderman * go fmt fixes * Update tests * Update tests Co-authored-by: Andrew Suderman --- pkg/dashboard/assets/css/main.css | 69 ++++---- pkg/dashboard/assets/js/filter.js | 154 +++++++++++++----- pkg/dashboard/assets/js/main.js | 5 + pkg/dashboard/assets/js/utilities.js | 17 ++ pkg/dashboard/helpers/helpers.go | 13 ++ pkg/dashboard/router.go | 6 +- pkg/dashboard/templates.go | 5 +- pkg/dashboard/templates/dashboard.gohtml | 41 ++--- pkg/dashboard/templates/filter.gohtml | 56 +++++-- pkg/dashboard/templates/head.gohtml | 2 +- pkg/dashboard/templates/namespace.gohtml | 2 + pkg/dashboard/templates/namespace_list.gohtml | 29 +--- pkg/dashboard/templates/navbar.gohtml | 27 --- pkg/dashboard/templates/navigation.gohtml | 46 ++++++ pkg/summary/constants_test.go | 2 + pkg/summary/summary.go | 17 +- 16 files changed, 304 insertions(+), 187 deletions(-) create mode 100644 pkg/dashboard/assets/js/main.js create mode 100644 pkg/dashboard/assets/js/utilities.js delete mode 100644 pkg/dashboard/templates/navbar.gohtml create mode 100644 pkg/dashboard/templates/navigation.gohtml diff --git a/pkg/dashboard/assets/css/main.css b/pkg/dashboard/assets/css/main.css index 4bd5298e..eaa38968 100644 --- a/pkg/dashboard/assets/css/main.css +++ b/pkg/dashboard/assets/css/main.css @@ -64,6 +64,10 @@ html { font-size: var(--step-0); } +body { + accent-color: var(--color-accent-2); +} + /* ----- ELEMENTS @@ -143,39 +147,6 @@ footer { background: var(--color-bg-2); } -form[role="search"] { - display: flex; -} - -form[role="search"] input[type="search"] { - background: none; - border: none; - border-bottom: 1px solid var(--color-accent-3); - color: var(--color-text-1); - flex: 1 1 30ch; - margin: 0; - padding: 0; -} - -form[role="search"] button[type="submit"] { - background: none; - border: 1px solid var(--color-accent-3); - border-radius: var(--gap--2); - margin: 0 0 0 var(--gap-0); -} - -form[role="search"] button[type="submit"]:active { - color: inherit; - transform: scale(var(--scale)); -} - -@media (prefers-reduced-motion) { - form[role="search"] button[type="submit"]:active { - background: var(--color-accent-3); - transform: none; - } -} - h1, h2, h3, @@ -210,6 +181,10 @@ h6 { font-size: var(--step-0); } +[hidden] { + display: none !important; +} + i.error { color: var(--color-negative); } @@ -347,6 +322,22 @@ COMPONENTS text-align: right; } +.controls { + border-block-end: 2px solid var(--color-accent-2); + margin-block: var(--gap-0); + padding-block-end: var(--gap-0); +} + +.control-block input { + display: block; + inline-size: min(30ch, 100%); + margin-block: var(--gap--2); +} + +.control-block .control-block__description { + display: inline; +} + .detailBadge { margin-left: calc(-1 * var(--gap-detail, var(--gap-0))); } @@ -471,14 +462,6 @@ COMPONENTS margin: var(--gap-1); } -.summary { - border-top: 2px solid var(--color-accent-2); - color: var(--color-accent-2); - margin-top: var(--gap-0); - margin-bottom: var(--gap-0); - padding-top: var(--gap-0); -} - .banner img { max-width: 750px; height: auto; @@ -491,6 +474,10 @@ UTILITIES ----- */ +body:not([data-javascript-available]) [data-javascript-required] { + display: none; +} + .indent { margin-left: var(--gap-1); } diff --git a/pkg/dashboard/assets/js/filter.js b/pkg/dashboard/assets/js/filter.js index 0a7025ef..f041133f 100644 --- a/pkg/dashboard/assets/js/filter.js +++ b/pkg/dashboard/assets/js/filter.js @@ -1,60 +1,124 @@ -(function () { - const formId = "js-filter-form"; - const containerId = "js-filter-container"; +import { + showElement, + hideElement +} from "./utilities.js"; - const form = document.getElementById(formId); - const filterInput = form?.querySelector("input[type='search']"); +const form = document.getElementById("js-filter-form"); +const container = document.getElementById("js-filter-container"); - const container = document.getElementById(containerId); - const potentialResults = container?.querySelectorAll("[data-filter]"); - const numPotentialResults = potentialResults?.length; +/* + These lookups simultaneously test that certain elements and attributes + required for accessibility are present +*/ +const filterInput = form?.querySelector("input[type='text']"); +const potentialResults = container?.querySelectorAll("[data-filter]"); - function showFilterResult(result) { - result.style.removeProperty("display"); - } +const outputVisual = form?.querySelector("output[aria-hidden]"); +const outputPolite = form?.querySelector("output[aria-live='polite']"); +const outputAlert = form?.querySelector("output[role='alert']"); - function hideFilterResult(result) { - result.style.display = "none"; - } +let statusDelay = null; + +// Test that all expected HTML is present +if (!form) { + console.error("Could not find filter form"); +} else if (!filterInput) { + hideElement(form); + console.error("Could not find filter input element, removed filter form"); +} else if (!container) { + hideElement(form); + console.error("Could not find filter results container, removed filter form"); +} else if (!outputVisual || !outputPolite || !outputAlert) { + hideElement(form); + console.error("Could not find all filter output elements, removed filter form"); +} else if (potentialResults.length === 0) { + hideElement(form); + console.error("No filterable entries found, removed filter form"); +} else { + // HTML was successfully set up, wire in JS + filterInput.addEventListener("input", runFilter); + + // Handle case where input value doesn't start empty (such as on page refresh) + runFilter(); +} - function updateResults() { - let filterTerm = filterInput.value; +function runFilter() { + updateResults(); + updateStatus(); +} - if (filterTerm) { - let regex = new RegExp(`${ filterTerm.trim().replace(/\s/g, "|") }`, "i"); +function updateResults() { + let filterTerm = filterInput.value; - for (let i = 0; i < numPotentialResults; i++) { - let result = potentialResults[i]; - let filterWithin = result.dataset.filter; + if (filterTerm) { + let regex = new RegExp(`${ filterTerm.trim().replace(/\s/g, "|") }`, "i"); - if (regex.test(filterWithin)) { - showFilterResult(result); - } else { - hideFilterResult(result); - } + for (const result of potentialResults) { + if (regex.test(result.dataset.filter)) { + showElement(result); + } else { + hideElement(result); } - } else { - clearFilter(); } + } else { + clearFilter(); } +} - function clearFilter() { - for (let i = 0; i < numPotentialResults; i++) { - showFilterResult(potentialResults[i]); - } +function clearFilter() { + for (const result of potentialResults) { + showElement(result); } +} - if (form && container) { - if (numPotentialResults === 0) { - form.style.display = "none"; - console.error("No filterable entries found, removed filter form"); - } else { - filterInput.addEventListener("input", updateResults); - - form.addEventListener("submit", function(event) { - event.preventDefault(); - updateResults(); - }) - } +function updateStatus() { + const numResults = container?.querySelectorAll("[data-filter]:not([hidden])").length; + + let message, type; + + if (!filterInput.value) { + message = `${potentialResults.length} namespaces found`; + type = "polite"; + } else if (numResults === 0) { + message = "No namespaces match filter"; + type = "alert"; + } else { + message = `Showing ${numResults} out of ${potentialResults.length} namespaces`; + type = "polite"; + } + + changeStatusMessage(message, type); +} + +function changeStatusMessage(message, type = "polite") { + if (statusDelay) { + window.clearTimeout(statusDelay); } -})(); + + outputVisual.textContent = message; + outputPolite.textContent = ""; + outputAlert.textContent = ""; + + /* + If you don't clear the content, then repeats of the same message aren't announced. + There must be a time gap between clearing and injecting new content for this to work. + Delay also: + - Helps make spoken announcements less disruptive by generating fewer of them + - Gives the screen reader a chance to finish announcing what's been typed, which will otherwise talk over these announcements (in MacOS/VoiceOver at least) + */ + statusDelay = window.setTimeout(() => { + switch (type) { + case "polite": + outputPolite.textContent = message; + outputAlert.textContent = ""; + break; + case "alert": + outputPolite.textContent = ""; + outputAlert.textContent = message; + break; + default: + outputPolite.textContent = "Error: There was a problem with the filter."; + outputAlert.textContent = ""; + } + }, 1000); +} diff --git a/pkg/dashboard/assets/js/main.js b/pkg/dashboard/assets/js/main.js new file mode 100644 index 00000000..e38e224e --- /dev/null +++ b/pkg/dashboard/assets/js/main.js @@ -0,0 +1,5 @@ +// For scripts that should always be run on every page + +import { setJavascriptAvailable } from "./utilities.js"; + +setJavascriptAvailable(); diff --git a/pkg/dashboard/assets/js/utilities.js b/pkg/dashboard/assets/js/utilities.js new file mode 100644 index 00000000..b3c5068b --- /dev/null +++ b/pkg/dashboard/assets/js/utilities.js @@ -0,0 +1,17 @@ +function setJavascriptAvailable() { + document.body.dataset.javascriptAvailable = true; +} + +function showElement(element) { + element.removeAttribute("hidden"); +} + +function hideElement(element) { + element.setAttribute("hidden", ""); +} + +export { + setJavascriptAvailable, + showElement, + hideElement +}; diff --git a/pkg/dashboard/helpers/helpers.go b/pkg/dashboard/helpers/helpers.go index 630bf6c4..9c23162a 100644 --- a/pkg/dashboard/helpers/helpers.go +++ b/pkg/dashboard/helpers/helpers.go @@ -15,6 +15,8 @@ package helpers import ( + "reflect" + "github.com/google/uuid" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -126,3 +128,14 @@ func ResourceName(name string) corev1.ResourceName { func GetUUID() string { return uuid.New().String() } + +func HasField(v interface{}, name string) bool { + rv := reflect.ValueOf(v) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } + if rv.Kind() != reflect.Struct { + return false + } + return rv.FieldByName(name).IsValid() +} diff --git a/pkg/dashboard/router.go b/pkg/dashboard/router.go index adfe6546..af69dcd2 100644 --- a/pkg/dashboard/router.go +++ b/pkg/dashboard/router.go @@ -15,9 +15,9 @@ package dashboard import ( + "k8s.io/klog/v2" "net/http" "path" - "k8s.io/klog/v2" packr "github.com/gobuffalo/packr/v2" "github.com/gorilla/mux" @@ -70,13 +70,13 @@ func GetRouter(setters ...Option) *mux.Router { // root router.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { // catch all other paths that weren't matched - if r.URL.Path != "/" && r.URL.Path != opts.basePath && r.URL.Path != opts.basePath + "/" { + if r.URL.Path != "/" && r.URL.Path != opts.basePath && r.URL.Path != opts.basePath+"/" { klog.Infof("404: %s", r.URL.Path) http.NotFound(w, r) return } - klog.Infof("redirecting to %v",path.Join(opts.basePath, "/namespaces")) + klog.Infof("redirecting to %v", path.Join(opts.basePath, "/namespaces")) // default redirect on root path http.Redirect(w, r, path.Join(opts.basePath, "/namespaces"), http.StatusMovedPermanently) }) diff --git a/pkg/dashboard/templates.go b/pkg/dashboard/templates.go index 238e4024..9109f594 100644 --- a/pkg/dashboard/templates.go +++ b/pkg/dashboard/templates.go @@ -23,7 +23,7 @@ const ( FooterTemplateName = "footer.gohtml" HeadTemplateName = "head.gohtml" NamespaceTemplateName = "namespace.gohtml" - NavbarTemplateName = "navbar.gohtml" + NavigationTemplateName = "navigation.gohtml" EmailTemplateName = "email.gohtml" ApiTokenTemplateName = "api_token.gohtml" CostSettingTemplateName = "cost_settings.gohtml" @@ -33,7 +33,7 @@ var ( // templates with these names are included by default in getTemplate() defaultIncludedTemplates = []string{ "head", - "navbar", + "navigation", "footer", } ) @@ -66,6 +66,7 @@ func getTemplate(name string, includedTemplates ...string) (*template.Template, "getStatusRange": helpers.GetStatusRange, "resourceName": helpers.ResourceName, "getUUID": helpers.GetUUID, + "hasField": helpers.HasField, }) // join the default templates and included templates diff --git a/pkg/dashboard/templates/dashboard.gohtml b/pkg/dashboard/templates/dashboard.gohtml index 7eb42c67..d81b4dee 100644 --- a/pkg/dashboard/templates/dashboard.gohtml +++ b/pkg/dashboard/templates/dashboard.gohtml @@ -2,6 +2,11 @@ {{ template "head" . }} + + {{ if gt (len .Data.VpaData.Namespaces) 1 }} + + {{ end }} + + + @@ -19,34 +26,7 @@
- {{ template "navbar" . }} - - - - {{ if gt (len .Data.VpaData.Namespaces) 1 }} - {{ template "filter" . }} - {{ end }} + {{ template "navigation" . }}
@@ -54,8 +34,13 @@ {{ template "email" . }} {{ template "api_token" . }} {{ template "cost_settings" . }} +

Namespace Details

+ {{ if gt (len .Data.VpaData.Namespaces) 1 }} + {{ template "filter" .Data.VpaData.Namespaces }} + {{ end }} +