Skip to content

Commit

Permalink
Refactor the toolbar html & css to improve its overall accessibility …
Browse files Browse the repository at this point in the history
…(bug 1171799, bug 1855695)

The first goal of this patch was to remove the tabindex because it helps
to improve overall a11y. That led to move some html elements associated
with the buttons which helped to position these elements relatively to their
buttons.
Consequently it was easy to change the toolbar height (configurable in Firefox
with the pref browser.uidensity): it's the second goal of this patch.
For a11y reasons we want to be able to change the height of the toolbar to make
the buttons larger.
  • Loading branch information
calixteman committed Sep 23, 2024
1 parent c90ce5d commit 6f40ed7
Show file tree
Hide file tree
Showing 9 changed files with 975 additions and 807 deletions.
2 changes: 1 addition & 1 deletion test/integration/find_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("find bar", () => {
await page.click("#viewFindButton");
await page.waitForSelector("#viewFindButton", { hidden: false });
await page.type("#findInput", "a");
await page.click("#findHighlightAll");
await page.click("#findHighlightAll + label");
await page.waitForSelector(".textLayer .highlight");

// The PDF file contains the text 'AB BA' in a monospace font on a
Expand Down
15 changes: 3 additions & 12 deletions web/annotation_editor_layer_builder.css
Original file line number Diff line number Diff line change
Expand Up @@ -1227,18 +1227,9 @@
}

#highlightParamsToolbarContainer {
height: auto;
padding-inline: 10px;
padding-block: 10px 16px;
gap: 16px;
display: flex;
flex-direction: column;
box-sizing: border-box;

.editorParamsLabel {
width: fit-content;
inset-inline-start: 0;
}
padding-inline: 10px;
padding-block-end: 12px;

.colorPicker {
display: flex;
Expand All @@ -1262,6 +1253,7 @@
align-items: center;
background: none;
flex: 0 0 auto;
padding: 0;

.swatch {
width: 24px;
Expand Down Expand Up @@ -1291,7 +1283,6 @@
align-self: stretch;

.editorParamsLabel {
width: 100%;
height: auto;
align-self: stretch;
}
Expand Down
6 changes: 5 additions & 1 deletion web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,11 @@ const PDFViewerApplication = {
}

if (!this.supportsIntegratedFind && appConfig.findBar) {
this.findBar = new PDFFindBar(appConfig.findBar, eventBus);
this.findBar = new PDFFindBar(
appConfig.findBar,
appConfig.principalContainer,
eventBus
);
}

if (appConfig.annotationEditorParams) {
Expand Down
9 changes: 6 additions & 3 deletions web/pdf_find_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ const MATCHES_COUNT_LIMIT = 1000;
* is done by PDFFindController.
*/
class PDFFindBar {
#mainContainer;

#resizeObserver = new ResizeObserver(this.#resizeObserverCallback.bind(this));

constructor(options, eventBus) {
constructor(options, mainContainer, eventBus) {
this.opened = false;

this.bar = options.bar;
Expand All @@ -42,6 +44,7 @@ class PDFFindBar {
this.findPreviousButton = options.findPreviousButton;
this.findNextButton = options.findNextButton;
this.eventBus = eventBus;
this.#mainContainer = mainContainer;

// Add event listeners to the DOM elements.
this.toggleButton.addEventListener("click", () => {
Expand Down Expand Up @@ -170,7 +173,7 @@ class PDFFindBar {
// - The width of the viewer itself changes.
// - The width of the findbar changes, by toggling the visibility
// (or localization) of find count/status messages.
this.#resizeObserver.observe(this.bar.parentNode);
this.#resizeObserver.observe(this.#mainContainer);
this.#resizeObserver.observe(this.bar);

this.opened = true;
Expand Down Expand Up @@ -200,7 +203,7 @@ class PDFFindBar {
}
}

#resizeObserverCallback(entries) {
#resizeObserverCallback() {
const { bar } = this;
// The find bar has an absolute position and thus the browser extends
// its width to the maximum possible width once the find bar does not fit
Expand Down
10 changes: 5 additions & 5 deletions web/toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
DEFAULT_SCALE_VALUE,
MAX_SCALE,
MIN_SCALE,
toggleCheckedBtn,
toggleExpandedBtn,
} from "./ui_utils.js";

/**
Expand Down Expand Up @@ -270,22 +270,22 @@ class Toolbar {
editorStampParamsToolbar,
} = this.#opts;

toggleCheckedBtn(
toggleExpandedBtn(
editorFreeTextButton,
mode === AnnotationEditorType.FREETEXT,
editorFreeTextParamsToolbar
);
toggleCheckedBtn(
toggleExpandedBtn(
editorHighlightButton,
mode === AnnotationEditorType.HIGHLIGHT,
editorHighlightParamsToolbar
);
toggleCheckedBtn(
toggleExpandedBtn(
editorInkButton,
mode === AnnotationEditorType.INK,
editorInkParamsToolbar
);
toggleCheckedBtn(
toggleExpandedBtn(
editorStampButton,
mode === AnnotationEditorType.STAMP,
editorStampParamsToolbar
Expand Down
4 changes: 2 additions & 2 deletions web/viewer-geckoview.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@

</head>

<body tabindex="1">
<body tabindex="0">
<div id="outerContainer">

<div id="mainContainer">

<div id="floatingToolbar">
<button id="download" class="toolbarButton" type="button" title="Download" tabindex="31" data-l10n-id="pdfjs-download-button">
<button id="download" class="toolbarButton" type="button" title="Download" tabindex="0" data-l10n-id="pdfjs-download-button">
<span data-l10n-id="pdfjs-download-button-label">Download</span>
</button>
</div>
Expand Down
Loading

0 comments on commit 6f40ed7

Please sign in to comment.