From d65169d7542ec88b13d773d1ad5efd02351d84c7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 11 Feb 2022 14:04:47 +0100 Subject: [PATCH] Try to improve a11y for the "button groups" in the SecondaryToolbar/Sidebar (issue 14526) *Please note:* I don't really know anything about a11y, hence it's possible that this patch either doesn't work correctly or at least isn't a complete solution. In both the SecondaryToolbar and the Sidebar we have "button groups" that functionally acts essentially like radio-buttons. Based on skimming through [this MDN article](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/radio_role) it thus appears that we should tag them as such, using `role="radiogroup"` and `role="radio"`, and then utilize the `aria-checked` attribute to indicate to a11y software which button is currently active. --- web/pdf_sidebar.js | 45 ++++++++--------- web/secondary_toolbar.js | 102 +++++++++++++++++++++------------------ web/viewer.html | 70 +++++++++++++++------------ 3 files changed, 113 insertions(+), 104 deletions(-) diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index f3f861c4e..e94e4d917 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -212,34 +212,29 @@ class PDFSidebar { // in order to prevent setting it to an invalid state. this.active = view; - // Update the CSS classes, for all buttons... - this.thumbnailButton.classList.toggle( - "toggled", - view === SidebarView.THUMBS - ); - this.outlineButton.classList.toggle( - "toggled", - view === SidebarView.OUTLINE - ); - this.attachmentsButton.classList.toggle( - "toggled", - view === SidebarView.ATTACHMENTS - ); - this.layersButton.classList.toggle("toggled", view === SidebarView.LAYERS); + const isThumbs = view === SidebarView.THUMBS, + isOutline = view === SidebarView.OUTLINE, + isAttachments = view === SidebarView.ATTACHMENTS, + isLayers = view === SidebarView.LAYERS; + + // Update the CSS classes (and aria attributes), for all buttons... + this.thumbnailButton.classList.toggle("toggled", isThumbs); + this.outlineButton.classList.toggle("toggled", isOutline); + this.attachmentsButton.classList.toggle("toggled", isAttachments); + this.layersButton.classList.toggle("toggled", isLayers); + + this.thumbnailButton.setAttribute("aria-checked", `${isThumbs}`); + this.outlineButton.setAttribute("aria-checked", `${isOutline}`); + this.attachmentsButton.setAttribute("aria-checked", `${isAttachments}`); + this.layersButton.setAttribute("aria-checked", `${isLayers}`); // ... and for all views. - this.thumbnailView.classList.toggle("hidden", view !== SidebarView.THUMBS); - this.outlineView.classList.toggle("hidden", view !== SidebarView.OUTLINE); - this.attachmentsView.classList.toggle( - "hidden", - view !== SidebarView.ATTACHMENTS - ); - this.layersView.classList.toggle("hidden", view !== SidebarView.LAYERS); + this.thumbnailView.classList.toggle("hidden", !isThumbs); + this.outlineView.classList.toggle("hidden", !isOutline); + this.attachmentsView.classList.toggle("hidden", !isAttachments); + this.layersView.classList.toggle("hidden", !isLayers); // Finally, update view-specific CSS classes. - this._outlineOptionsContainer.classList.toggle( - "hidden", - view !== SidebarView.OUTLINE - ); + this._outlineOptionsContainer.classList.toggle("hidden", !isOutline); if (forceOpen && !this.isOpen) { this.open(); diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index fc8369e70..fd72da7aa 100644 --- a/web/secondary_toolbar.js +++ b/web/secondary_toolbar.js @@ -222,53 +222,58 @@ class SecondaryToolbar { } } - #bindCursorToolsListener(buttons) { + #bindCursorToolsListener({ cursorSelectToolButton, cursorHandToolButton }) { this.eventBus._on("cursortoolchanged", function ({ tool }) { - buttons.cursorSelectToolButton.classList.toggle( - "toggled", - tool === CursorTool.SELECT - ); - buttons.cursorHandToolButton.classList.toggle( - "toggled", - tool === CursorTool.HAND - ); + const isSelect = tool === CursorTool.SELECT, + isHand = tool === CursorTool.HAND; + + cursorSelectToolButton.classList.toggle("toggled", isSelect); + cursorHandToolButton.classList.toggle("toggled", isHand); + + cursorSelectToolButton.setAttribute("aria-checked", `${isSelect}`); + cursorHandToolButton.setAttribute("aria-checked", `${isHand}`); }); } - #bindScrollModeListener(buttons) { + #bindScrollModeListener({ + scrollPageButton, + scrollVerticalButton, + scrollHorizontalButton, + scrollWrappedButton, + spreadNoneButton, + spreadOddButton, + spreadEvenButton, + }) { const scrollModeChanged = ({ mode }) => { - buttons.scrollPageButton.classList.toggle( - "toggled", - mode === ScrollMode.PAGE - ); - buttons.scrollVerticalButton.classList.toggle( - "toggled", - mode === ScrollMode.VERTICAL - ); - buttons.scrollHorizontalButton.classList.toggle( - "toggled", - mode === ScrollMode.HORIZONTAL - ); - buttons.scrollWrappedButton.classList.toggle( - "toggled", - mode === ScrollMode.WRAPPED - ); + const isPage = mode === ScrollMode.PAGE, + isVertical = mode === ScrollMode.VERTICAL, + isHorizontal = mode === ScrollMode.HORIZONTAL, + isWrapped = mode === ScrollMode.WRAPPED; + + scrollPageButton.classList.toggle("toggled", isPage); + scrollVerticalButton.classList.toggle("toggled", isVertical); + scrollHorizontalButton.classList.toggle("toggled", isHorizontal); + scrollWrappedButton.classList.toggle("toggled", isWrapped); + + scrollPageButton.setAttribute("aria-checked", `${isPage}`); + scrollVerticalButton.setAttribute("aria-checked", `${isVertical}`); + scrollHorizontalButton.setAttribute("aria-checked", `${isHorizontal}`); + scrollWrappedButton.setAttribute("aria-checked", `${isWrapped}`); // Permanently *disable* the Scroll buttons when PAGE-scrolling is being // enforced for *very* long/large documents; please see the `BaseViewer`. const forceScrollModePage = this.pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE; - buttons.scrollPageButton.disabled = forceScrollModePage; - buttons.scrollVerticalButton.disabled = forceScrollModePage; - buttons.scrollHorizontalButton.disabled = forceScrollModePage; - buttons.scrollWrappedButton.disabled = forceScrollModePage; + scrollPageButton.disabled = forceScrollModePage; + scrollVerticalButton.disabled = forceScrollModePage; + scrollHorizontalButton.disabled = forceScrollModePage; + scrollWrappedButton.disabled = forceScrollModePage; // Temporarily *disable* the Spread buttons when horizontal scrolling is // enabled, since the non-default Spread modes doesn't affect the layout. - const isScrollModeHorizontal = mode === ScrollMode.HORIZONTAL; - buttons.spreadNoneButton.disabled = isScrollModeHorizontal; - buttons.spreadOddButton.disabled = isScrollModeHorizontal; - buttons.spreadEvenButton.disabled = isScrollModeHorizontal; + spreadNoneButton.disabled = isHorizontal; + spreadOddButton.disabled = isHorizontal; + spreadEvenButton.disabled = isHorizontal; }; this.eventBus._on("scrollmodechanged", scrollModeChanged); @@ -279,20 +284,23 @@ class SecondaryToolbar { }); } - #bindSpreadModeListener(buttons) { + #bindSpreadModeListener({ + spreadNoneButton, + spreadOddButton, + spreadEvenButton, + }) { function spreadModeChanged({ mode }) { - buttons.spreadNoneButton.classList.toggle( - "toggled", - mode === SpreadMode.NONE - ); - buttons.spreadOddButton.classList.toggle( - "toggled", - mode === SpreadMode.ODD - ); - buttons.spreadEvenButton.classList.toggle( - "toggled", - mode === SpreadMode.EVEN - ); + const isNone = mode === SpreadMode.NONE, + isOdd = mode === SpreadMode.ODD, + isEven = mode === SpreadMode.EVEN; + + spreadNoneButton.classList.toggle("toggled", isNone); + spreadOddButton.classList.toggle("toggled", isOdd); + spreadEvenButton.classList.toggle("toggled", isEven); + + spreadNoneButton.setAttribute("aria-checked", `${isNone}`); + spreadOddButton.setAttribute("aria-checked", `${isOdd}`); + spreadEvenButton.setAttribute("aria-checked", `${isEven}`); } this.eventBus._on("spreadmodechanged", spreadModeChanged); diff --git a/web/viewer.html b/web/viewer.html index a6de436e5..26c208d34 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -74,17 +74,17 @@ See https://github.com/adobe-type-tools/cmap-resources
-
- - - -
@@ -191,39 +191,45 @@ See https://github.com/adobe-type-tools/cmap-resources
- - +
+ + +
- - - - +
+ + + + +
- - - +
+ + + +