Skip to content

Commit

Permalink
Merge pull request #18971 from Snuffleupagus/AltText-fluent
Browse files Browse the repository at this point in the history 
[Editor] Utilize Fluent "better" when localizing the AltText
  • Loading branch information
@Snuffleupagus
Snuffleupagus authored Oct 28, 2024
2 parents 9108848 + ee812b5 commit 93961e2
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 58 deletions.
11 changes: 10 additions & 1 deletion l10n/en-US/viewer.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,12 @@ pdfjs-ink-canvas =
## Alt-text dialog

# Alternative text (alt text) helps when people can't see the image.
pdfjs-editor-alt-text-button =
.aria-label = Alt text
pdfjs-editor-alt-text-button-label = Alt text
pdfjs-editor-alt-text-edit-button-label = Edit alt text
pdfjs-editor-alt-text-edit-button =
.aria-label = Edit alt text
pdfjs-editor-alt-text-dialog-label = Choose an option
pdfjs-editor-alt-text-dialog-description = Alt text (alternative text) helps when people can’t see the image or when it doesn’t load.
pdfjs-editor-alt-text-add-description-label = Add a description
Expand Down Expand Up @@ -457,12 +460,18 @@ pdfjs-editor-new-alt-text-ai-model-downloading-progress = Downloading alt text A
.aria-valuetext = Downloading alt text AI model ({ $downloadedSize } of { $totalSize } MB)
# This is a button that users can click to edit the alt text they have already added.
pdfjs-editor-new-alt-text-added-button =
.aria-label = Alt text added
pdfjs-editor-new-alt-text-added-button-label = Alt text added
# This is a button that users can click to open the alt text editor and add alt text when it is not present.
pdfjs-editor-new-alt-text-missing-button =
.aria-label = Missing alt text
pdfjs-editor-new-alt-text-missing-button-label = Missing alt text
# This is a button that opens up the alt text modal where users should review the alt text that was automatically generated.
pdfjs-editor-new-alt-text-to-review-button =
.aria-label = Review alt text
pdfjs-editor-new-alt-text-to-review-button-label = Review alt text
# "Created automatically" is a prefix that will be added to the beginning of any alt text that has been automatically generated. After the colon, the user will see/hear the actual alt text description. If the alt text has been edited by a human, this prefix will not appear.
Expand Down
85 changes: 45 additions & 40 deletions src/display/editor/alt_text.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class AltText {

#altTextButton = null;

#altTextButtonLabel = null;

#altTextTooltip = null;

#altTextTooltipTimeout = null;
Expand All @@ -40,38 +42,46 @@ class AltText {

static #l10nNewButton = null;

static _l10nPromise = null;
static _l10n = null;

constructor(editor) {
this.#editor = editor;
this.#useNewAltTextFlow = editor._uiManager.useNewAltTextFlow;

AltText.#l10nNewButton ||= Object.freeze({
added: "pdfjs-editor-new-alt-text-added-button-label",
missing: "pdfjs-editor-new-alt-text-missing-button-label",
review: "pdfjs-editor-new-alt-text-to-review-button-label",
added: "pdfjs-editor-new-alt-text-added-button",
"added-label": "pdfjs-editor-new-alt-text-added-button-label",
missing: "pdfjs-editor-new-alt-text-missing-button",
"missing-label": "pdfjs-editor-new-alt-text-missing-button-label",
review: "pdfjs-editor-new-alt-text-to-review-button",
"review-label": "pdfjs-editor-new-alt-text-to-review-button-label",
});
}

static initialize(l10nPromise) {
AltText._l10nPromise ||= l10nPromise;
static initialize(l10n) {
AltText._l10n ??= l10n;
}

async render() {
const altText = (this.#altTextButton = document.createElement("button"));
altText.className = "altText";
let msg;
altText.tabIndex = "0";

const label = (this.#altTextButtonLabel = document.createElement("span"));
altText.append(label);

if (this.#useNewAltTextFlow) {
altText.classList.add("new");
msg = await AltText._l10nPromise.get(AltText.#l10nNewButton.missing);
} else {
msg = await AltText._l10nPromise.get(
"pdfjs-editor-alt-text-button-label"
altText.setAttribute("data-l10n-id", AltText.#l10nNewButton.missing);
label.setAttribute(
"data-l10n-id",
AltText.#l10nNewButton["missing-label"]
);
} else {
altText.setAttribute("data-l10n-id", "pdfjs-editor-alt-text-button");
label.setAttribute("data-l10n-id", "pdfjs-editor-alt-text-button-label");
}
altText.textContent = msg;
altText.setAttribute("aria-label", msg);
altText.tabIndex = "0";

const signal = this.#editor._uiManager._signal;
altText.addEventListener("contextmenu", noContextMenu, { signal });
altText.addEventListener("pointerdown", event => event.stopPropagation(), {
Expand Down Expand Up @@ -144,9 +154,10 @@ class AltText {
return;
}
this.#guessedText = guessedText;
this.#textWithDisclaimer = await AltText._l10nPromise.get(
"pdfjs-editor-new-alt-text-generated-alt-text-with-disclaimer"
)({ generatedAltText: guessedText });
this.#textWithDisclaimer = await AltText._l10n.get(
"pdfjs-editor-new-alt-text-generated-alt-text-with-disclaimer",
{ generatedAltText: guessedText }
);
this.#setState();
}

Expand Down Expand Up @@ -229,6 +240,7 @@ class AltText {
destroy() {
this.#altTextButton?.remove();
this.#altTextButton = null;
this.#altTextButtonLabel = null;
this.#altTextTooltip = null;
this.#badge?.remove();
this.#badge = null;
Expand All @@ -242,19 +254,12 @@ class AltText {

if (this.#useNewAltTextFlow) {
button.classList.toggle("done", !!this.#altText);
AltText._l10nPromise
.get(AltText.#l10nNewButton[this.#label])
.then(msg => {
button.setAttribute("aria-label", msg);
// We can't just use button.textContent here, because it would remove
// the existing tooltip element.
for (const child of button.childNodes) {
if (child.nodeType === Node.TEXT_NODE) {
child.textContent = msg;
break;
}
}
});
button.setAttribute("data-l10n-id", AltText.#l10nNewButton[this.#label]);

this.#altTextButtonLabel?.setAttribute(
"data-l10n-id",
AltText.#l10nNewButton[`${this.#label}-label`]
);
if (!this.#altText) {
this.#altTextTooltip?.remove();
return;
Expand All @@ -266,11 +271,7 @@ class AltText {
return;
}
button.classList.add("done");
AltText._l10nPromise
.get("pdfjs-editor-alt-text-edit-button-label")
.then(msg => {
button.setAttribute("aria-label", msg);
});
button.setAttribute("data-l10n-id", "pdfjs-editor-alt-text-edit-button");
}

let tooltip = this.#altTextTooltip;
Expand Down Expand Up @@ -315,11 +316,15 @@ class AltText {
{ signal }
);
}
tooltip.innerText = this.#altTextDecorative
? await AltText._l10nPromise.get(
"pdfjs-editor-alt-text-decorative-tooltip"
)
: this.#altText;
if (this.#altTextDecorative) {
tooltip.setAttribute(
"data-l10n-id",
"pdfjs-editor-alt-text-decorative-tooltip"
);
} else {
tooltip.removeAttribute("data-l10n-id");
tooltip.textContent = this.#altText;
}

if (!tooltip.parentNode) {
button.append(tooltip);
Expand Down
21 changes: 4 additions & 17 deletions src/display/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class AnnotationEditor {

_focusEventsAllowed = true;

static _l10nPromise = null;
static _l10n = null;

static _l10nResizer = null;

Expand Down Expand Up @@ -210,6 +210,8 @@ class AnnotationEditor {
* @param {Object} l10n
*/
static initialize(l10n, _uiManager) {
AnnotationEditor._l10n ??= l10n;

AnnotationEditor._l10nResizer ||= Object.freeze({
topLeft: "pdfjs-editor-resizer-top-left",
topMiddle: "pdfjs-editor-resizer-top-middle",
Expand All @@ -221,21 +223,6 @@ class AnnotationEditor {
middleLeft: "pdfjs-editor-resizer-middle-left",
});

AnnotationEditor._l10nPromise ||= new Map([
...[
"pdfjs-editor-alt-text-button-label",
"pdfjs-editor-alt-text-edit-button-label",
"pdfjs-editor-alt-text-decorative-tooltip",
"pdfjs-editor-new-alt-text-added-button-label",
"pdfjs-editor-new-alt-text-missing-button-label",
"pdfjs-editor-new-alt-text-to-review-button-label",
].map(str => [str, l10n.get(str)]),
...[
// Strings that need l10n-arguments.
"pdfjs-editor-new-alt-text-generated-alt-text-with-disclaimer",
].map(str => [str, l10n.get.bind(l10n, str)]),
]);

if (AnnotationEditor._borderLineWidth !== -1) {
return;
}
Expand Down Expand Up @@ -1003,7 +990,7 @@ class AnnotationEditor {
if (this.#altText) {
return;
}
AltText.initialize(AnnotationEditor._l10nPromise);
AltText.initialize(AnnotationEditor._l10n);
this.#altText = new AltText(this);
if (this.#accessibilityData) {
this.#altText.data = this.#accessibilityData;
Expand Down
11 changes: 11 additions & 0 deletions test/integration/stamp_editor_spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ const copyImage = async (page, imagePath, number) => {
await waitForImage(page, getEditorSelector(number));
};

async function waitForTranslation(page) {
return page.evaluate(async () => {
await new Promise(resolve => {
window.requestAnimationFrame(resolve);
});
});
}

const switchToStamp = switchToEditor.bind(null, "Stamp");

describe("Stamp Editor", () => {
Expand Down Expand Up @@ -987,6 +995,7 @@ describe("Stamp Editor", () => {
const buttonSelector = `${editorSelector} button.altText.new`;
await page.waitForSelector(buttonSelector, { visible: true });

await waitForTranslation(page);
// Check the text in the button.
let text = await page.evaluate(
sel => document.querySelector(sel).textContent,
Expand Down Expand Up @@ -1036,6 +1045,7 @@ describe("Stamp Editor", () => {
await waitForSelectedEditor(page, editorSelector);
await page.waitForSelector(buttonSelector, { visible: true });

await waitForTranslation(page);
// Check the text in the button.
text = await page.evaluate(
sel => document.querySelector(sel).textContent,
Expand Down Expand Up @@ -1078,6 +1088,7 @@ describe("Stamp Editor", () => {
await page.click("#newAltTextSave");
await page.waitForSelector("#newAltTextDialog", { visible: false });

await waitForTranslation(page);
// Check the text in the button.
text = await page.evaluate(
sel => document.querySelector(sel).firstChild.textContent,
Expand Down

0 comments on commit 93961e2

Please sign in to comment.