Skip to content

Commit

Permalink
Try to improve a11y for the "button groups" in the SecondaryToolbar/S…
Browse files Browse the repository at this point in the history 
…idebar (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.
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Mar 6, 2022
1 parent c1f73b1 commit d65169d
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 104 deletions.
45 changes: 20 additions & 25 deletions web/pdf_sidebar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
102 changes: 55 additions & 47 deletions web/secondary_toolbar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
70 changes: 38 additions & 32 deletions web/viewer.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,17 @@
<div id="sidebarContainer">
<div id="toolbarSidebar">
<div id="toolbarSidebarLeft">
<div class="splitToolbarButton toggled">
<button id="viewThumbnail" class="toolbarButton toggled" title="Show Thumbnails" tabindex="2" data-l10n-id="thumbs">
<div id="sidebarViewButtons" class="splitToolbarButton toggled" role="radiogroup">
<button id="viewThumbnail" class="toolbarButton toggled" title="Show Thumbnails" tabindex="2" data-l10n-id="thumbs" role="radio" aria-checked="true" aria-controls="thumbnailView">
<span data-l10n-id="thumbs_label">Thumbnails</span>
</button>
<button id="viewOutline" class="toolbarButton" title="Show Document Outline (double-click to expand/collapse all items)" tabindex="3" data-l10n-id="document_outline">
<button id="viewOutline" class="toolbarButton" title="Show Document Outline (double-click to expand/collapse all items)" tabindex="3" data-l10n-id="document_outline" role="radio" aria-checked="false" aria-controls="outlineView">
<span data-l10n-id="document_outline_label">Document Outline</span>
</button>
<button id="viewAttachments" class="toolbarButton" title="Show Attachments" tabindex="4" data-l10n-id="attachments">
<button id="viewAttachments" class="toolbarButton" title="Show Attachments" tabindex="4" data-l10n-id="attachments" role="radio" aria-checked="false" aria-controls="attachmentsView">
<span data-l10n-id="attachments_label">Attachments</span>
</button>
<button id="viewLayers" class="toolbarButton" title="Show Layers (double-click to reset all layers to the default state)" tabindex="5" data-l10n-id="layers">
<button id="viewLayers" class="toolbarButton" title="Show Layers (double-click to reset all layers to the default state)" tabindex="5" data-l10n-id="layers" role="radio" aria-checked="false" aria-controls="layersView">
<span data-l10n-id="layers_label">Layers</span>
</button>
</div>
Expand Down Expand Up @@ -191,39 +191,45 @@

<div class="horizontalToolbarSeparator"></div>

<button id="cursorSelectTool" class="secondaryToolbarButton selectTool toggled" title="Enable Text Selection Tool" tabindex="60" data-l10n-id="cursor_text_select_tool">
<span data-l10n-id="cursor_text_select_tool_label">Text Selection Tool</span>
</button>
<button id="cursorHandTool" class="secondaryToolbarButton handTool" title="Enable Hand Tool" tabindex="61" data-l10n-id="cursor_hand_tool">
<span data-l10n-id="cursor_hand_tool_label">Hand Tool</span>
</button>
<div id="cursorToolButtons" role="radiogroup">
<button id="cursorSelectTool" class="secondaryToolbarButton selectTool toggled" title="Enable Text Selection Tool" tabindex="60" data-l10n-id="cursor_text_select_tool" role="radio" aria-checked="true">
<span data-l10n-id="cursor_text_select_tool_label">Text Selection Tool</span>
</button>
<button id="cursorHandTool" class="secondaryToolbarButton handTool" title="Enable Hand Tool" tabindex="61" data-l10n-id="cursor_hand_tool" role="radio" aria-checked="false">
<span data-l10n-id="cursor_hand_tool_label">Hand Tool</span>
</button>
</div>

<div class="horizontalToolbarSeparator"></div>

<button id="scrollPage" class="secondaryToolbarButton scrollPage" title="Use Page Scrolling" tabindex="62" data-l10n-id="scroll_page">
<span data-l10n-id="scroll_page_label">Page Scrolling</span>
</button>
<button id="scrollVertical" class="secondaryToolbarButton scrollVertical toggled" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="scroll_vertical">
<span data-l10n-id="scroll_vertical_label">Vertical Scrolling</span>
</button>
<button id="scrollHorizontal" class="secondaryToolbarButton scrollHorizontal" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="scroll_horizontal">
<span data-l10n-id="scroll_horizontal_label">Horizontal Scrolling</span>
</button>
<button id="scrollWrapped" class="secondaryToolbarButton scrollWrapped" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="scroll_wrapped">
<span data-l10n-id="scroll_wrapped_label">Wrapped Scrolling</span>
</button>
<div id="scrollModeButtons" role="radiogroup">
<button id="scrollPage" class="secondaryToolbarButton scrollPage" title="Use Page Scrolling" tabindex="62" data-l10n-id="scroll_page" role="radio" aria-checked="false">
<span data-l10n-id="scroll_page_label">Page Scrolling</span>
</button>
<button id="scrollVertical" class="secondaryToolbarButton scrollVertical toggled" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="scroll_vertical" role="radio" aria-checked="true">
<span data-l10n-id="scroll_vertical_label" >Vertical Scrolling</span>
</button>
<button id="scrollHorizontal" class="secondaryToolbarButton scrollHorizontal" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="scroll_horizontal" role="radio" aria-checked="false">
<span data-l10n-id="scroll_horizontal_label">Horizontal Scrolling</span>
</button>
<button id="scrollWrapped" class="secondaryToolbarButton scrollWrapped" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="scroll_wrapped" role="radio" aria-checked="false">
<span data-l10n-id="scroll_wrapped_label">Wrapped Scrolling</span>
</button>
</div>

<div class="horizontalToolbarSeparator"></div>

<button id="spreadNone" class="secondaryToolbarButton spreadNone toggled" title="Do not join page spreads" tabindex="66" data-l10n-id="spread_none">
<span data-l10n-id="spread_none_label">No Spreads</span>
</button>
<button id="spreadOdd" class="secondaryToolbarButton spreadOdd" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="spread_odd">
<span data-l10n-id="spread_odd_label">Odd Spreads</span>
</button>
<button id="spreadEven" class="secondaryToolbarButton spreadEven" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="spread_even">
<span data-l10n-id="spread_even_label">Even Spreads</span>
</button>
<div id="spreadModeButtons" role="radiogroup">
<button id="spreadNone" class="secondaryToolbarButton spreadNone toggled" title="Do not join page spreads" tabindex="66" data-l10n-id="spread_none" role="radio" aria-checked="true">
<span data-l10n-id="spread_none_label">No Spreads</span>
</button>
<button id="spreadOdd" class="secondaryToolbarButton spreadOdd" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="spread_odd" role="radio" aria-checked="false">
<span data-l10n-id="spread_odd_label">Odd Spreads</span>
</button>
<button id="spreadEven" class="secondaryToolbarButton spreadEven" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="spread_even" role="radio" aria-checked="false">
<span data-l10n-id="spread_even_label">Even Spreads</span>
</button>
</div>

<div class="horizontalToolbarSeparator"></div>

Expand Down

0 comments on commit d65169d

Please sign in to comment.