Skip to content

Commit

Permalink
[Editor] Don't show the alt-text button when the alt-text dialog is v…
Browse files Browse the repository at this point in the history
…isible

This way, the button doens't cover the image.
  • Loading branch information
calixteman committed Sep 28, 2023
1 parent 3072efa commit be31723
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 16 deletions.
30 changes: 18 additions & 12 deletions src/display/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -840,18 +840,16 @@ class AnnotationEditor {
altText.tabIndex = "0";
altText.addEventListener("contextmenu", noContextMenu);
altText.addEventListener("pointerdown", event => event.stopPropagation());
altText.addEventListener(
"click",
event => {
event.preventDefault();
this._uiManager.editAltText(this);
},
{ capture: true }
);

const onClick = (event, keyboard = false) => {
this.#altTextButton.hidden = true;
event.preventDefault();
this._uiManager.editAltText(this, /* isFromKeyboard */ keyboard);
};
altText.addEventListener("click", onClick, { capture: true });
altText.addEventListener("keydown", event => {
if (event.target === altText && event.key === "Enter") {
event.preventDefault();
this._uiManager.editAltText(this);
onClick(event, true);
}
});
this.#setAltTextButtonState();
Expand All @@ -877,12 +875,13 @@ class AnnotationEditor {
this.#altTextTooltip?.remove();
return;
}
button.classList.add("done");

AnnotationEditor._l10nPromise
.get("editor_alt_text_edit_button_label")
.then(msg => {
button.setAttribute("aria-label", msg);
});

let tooltip = this.#altTextTooltip;
if (!tooltip) {
this.#altTextTooltip = tooltip = document.createElement("span");
Expand Down Expand Up @@ -916,7 +915,6 @@ class AnnotationEditor {
this.#altTextTooltip?.classList.remove("show");
});
}
button.classList.add("done");
tooltip.innerText = this.#altTextDecorative
? await AnnotationEditor._l10nPromise.get(
"editor_alt_text_decorative_tooltip"
Expand All @@ -939,6 +937,14 @@ class AnnotationEditor {
this.#altTextButton.disabled = !enabled;
}

altTextFinish(wasFromKeyboard) {
if (!this.#altTextButton) {
return;
}
this.#altTextButton.hidden = false;
this.#altTextButton.focus({ focusVisible: wasFromKeyboard });
}

getClientDimensions() {
return this.div.getBoundingClientRect();
}
Expand Down
4 changes: 2 additions & 2 deletions src/display/editor/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -768,8 +768,8 @@ class AnnotationEditorUIManager {
);
}

editAltText(editor) {
this.#altTextManager?.editAltText(this, editor);
editAltText(editor, isFromKeyboard) {
this.#altTextManager?.editAltText(this, editor, isFromKeyboard);
}

onPageChanging({ pageNumber }) {
Expand Down
29 changes: 28 additions & 1 deletion test/integration/stamp_editor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ describe("Stamp Editor", () => {
// Click on the alt-text button.
await page.click(buttonSelector);

// Check that the alt-text button has been hidden.
await page.waitForSelector(`${buttonSelector}[hidden]`);

// Wait for the alt-text dialog to be visible.
await page.waitForSelector("#altTextDialog", { visible: true });

Expand All @@ -254,7 +257,7 @@ describe("Stamp Editor", () => {
await page.click(saveButtonSelector);

// Wait for the alt-text button to have the correct icon.
await page.waitForSelector(`${buttonSelector}.done`);
await page.waitForSelector(`${buttonSelector}:not([hidden]).done`);

// Hover the button.
await page.hover(buttonSelector);
Expand Down Expand Up @@ -350,6 +353,30 @@ describe("Stamp Editor", () => {
sel => document.querySelector(sel) === null,
tooltipSelector
);

// We check that the alt-text button works correctly with the
// keyboard.
await page.evaluate(sel => {
document.getElementById("viewerContainer").focus();
return new Promise(resolve => {
setTimeout(() => {
const el = document.querySelector(sel);
el.addEventListener("focus", resolve, { once: true });
el.focus({ focusVisible: true });
}, 0);
});
}, buttonSelector);
await (browserName === "chrome"
? page.waitForSelector(`${buttonSelector}:focus`)
: page.waitForSelector(`${buttonSelector}:focus-visible`));
await page.keyboard.press("Enter");
await page.waitForSelector(`${buttonSelector}[hidden]`);
await page.waitForSelector("#altTextDialog", { visible: true });
await page.keyboard.press("Escape");
await page.waitForSelector(`${buttonSelector}:not([hidden])`);
await (browserName === "chrome"
? page.waitForSelector(`${buttonSelector}:focus`)
: page.waitForSelector(`${buttonSelector}:focus-visible`));
})
);
});
Expand Down
6 changes: 5 additions & 1 deletion web/alt_text_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class AltTextManager {

#hasUsedPointer = false;

#isFromKeyboard = false;

#optionDescription;

#optionDecorative;
Expand Down Expand Up @@ -134,7 +136,7 @@ class AltTextManager {
this.#dialog.append(svg);
}

async editAltText(uiManager, editor) {
async editAltText(uiManager, editor, isFromKeyboard) {
if (this.#currentEditor || !editor) {
return;
}
Expand All @@ -157,6 +159,7 @@ class AltTextManager {
this.#previousAltText = this.#textarea.value = altText?.trim() || "";
this.#updateUIState();

this.#isFromKeyboard = isFromKeyboard;
this.#currentEditor = editor;
this.#uiManager = uiManager;
this.#uiManager.removeEditListeners();
Expand Down Expand Up @@ -264,6 +267,7 @@ class AltTextManager {
this.#removeOnClickListeners();
this.#uiManager?.addEditListeners();
this.#eventBus._off("resize", this.#boundSetPosition);
this.#currentEditor.altTextFinish(this.#isFromKeyboard);
this.#currentEditor = null;
this.#uiManager = null;
}
Expand Down
1 change: 1 addition & 0 deletions web/annotation_editor_layer_builder.css
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@
min-width: 88px;
z-index: 1;
pointer-events: all;
opacity: var(--alt-text-button-opacity);

color: var(--alt-text-fg-color);
font: menu;
Expand Down

0 comments on commit be31723

Please sign in to comment.