Skip to content

Commit b079148

Browse files
committed
[Editor] Remove a popup from the DOM when it's deleted
1 parent 2e0f1ec commit b079148

File tree

4 files changed

+103
-16
lines changed

4 files changed

+103
-16
lines changed

src/display/annotation_layer.js

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,6 +2125,8 @@ class PopupElement {
21252125

21262126
#popup = null;
21272127

2128+
#popupAbortController = null;
2129+
21282130
#position = null;
21292131

21302132
#rect = null;
@@ -2166,18 +2168,7 @@ class PopupElement {
21662168
this.#dateObj = PDFDateString.toDateObject(modificationDate);
21672169

21682170
this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
2169-
// Attach the event listeners to the trigger element.
2170-
for (const element of this.trigger) {
2171-
element.addEventListener("click", this.#boundToggle);
2172-
element.addEventListener("mouseenter", this.#boundShow);
2173-
element.addEventListener("mouseleave", this.#boundHide);
2174-
element.classList.add("popupTriggerArea");
2175-
}
2176-
2177-
// Attach the event listener to toggle the popup with the keyboard.
2178-
for (const element of elements) {
2179-
element.container?.addEventListener("keydown", this.#boundKeyDown);
2180-
}
2171+
this.#addEventListeners();
21812172

21822173
this.#container.hidden = true;
21832174
if (open) {
@@ -2195,6 +2186,29 @@ class PopupElement {
21952186
}
21962187
}
21972188

2189+
#addEventListeners() {
2190+
if (this.#popupAbortController) {
2191+
return;
2192+
}
2193+
this.#popupAbortController = new AbortController();
2194+
const { signal } = this.#popupAbortController;
2195+
2196+
// Attach the event listeners to the trigger element.
2197+
for (const element of this.trigger) {
2198+
element.addEventListener("click", this.#boundToggle, { signal });
2199+
element.addEventListener("mouseenter", this.#boundShow, { signal });
2200+
element.addEventListener("mouseleave", this.#boundHide, { signal });
2201+
element.classList.add("popupTriggerArea");
2202+
}
2203+
2204+
// Attach the event listener to toggle the popup with the keyboard.
2205+
for (const element of this.#elements) {
2206+
element.container?.addEventListener("keydown", this.#boundKeyDown, {
2207+
signal,
2208+
});
2209+
}
2210+
}
2211+
21982212
render() {
21992213
if (this.#popup) {
22002214
return;
@@ -2338,7 +2352,12 @@ class PopupElement {
23382352
}
23392353
}
23402354

2341-
updateEdited({ rect, popupContent }) {
2355+
updateEdited({ rect, popupContent, deleted }) {
2356+
if (deleted) {
2357+
this.remove();
2358+
return;
2359+
}
2360+
this.#addEventListeners();
23422361
this.#updates ||= {
23432362
contentsObj: this.#contentsObj,
23442363
richText: this.#richText,
@@ -2366,6 +2385,18 @@ class PopupElement {
23662385
this.#position = null;
23672386
}
23682387

2388+
remove() {
2389+
this.#popupAbortController?.abort();
2390+
this.#popupAbortController = null;
2391+
this.#popup?.remove();
2392+
this.#popup = null;
2393+
this.#wasVisible = false;
2394+
this.#pinned = false;
2395+
for (const element of this.trigger) {
2396+
element.classList.remove("popupTriggerArea");
2397+
}
2398+
}
2399+
23692400
#setPosition() {
23702401
if (this.#position !== null) {
23712402
return;
@@ -2465,6 +2496,7 @@ class PopupElement {
24652496
}
24662497

24672498
maybeShow() {
2499+
this.#addEventListeners();
24682500
if (!this.#wasVisible) {
24692501
return;
24702502
}

src/display/editor/annotation_editor_layer.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,9 @@ class AnnotationEditorLayer {
359359
for (const editable of editables) {
360360
const { id } = editable.data;
361361
if (this.#uiManager.isDeletedAnnotationElement(id)) {
362+
editable.updateEdited({
363+
deleted: true,
364+
});
362365
continue;
363366
}
364367
let editor = resetAnnotations.get(id);

src/display/editor/freetext.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,6 @@ class FreeTextEditor extends AnnotationEditor {
860860
/** @inheritdoc */
861861
renderAnnotationElement(annotation) {
862862
const content = super.renderAnnotationElement(annotation);
863-
if (this.deleted) {
864-
return content;
865-
}
866863
const { style } = content;
867864
style.fontSize = `calc(${this.#fontSize}px * var(--total-scale-factor))`;
868865
style.color = this.#color;

test/integration/freetext_editor_spec.mjs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,61 @@ describe("FreeText Editor", () => {
10861086
})
10871087
);
10881088
});
1089+
1090+
it("must delete an existing annotation with a popup", async () => {
1091+
await Promise.all(
1092+
pages.map(async ([browserName, page]) => {
1093+
await page.click("[data-annotation-id='26R']");
1094+
// Wait for the popup to be displayed.
1095+
const popupSelector = "[data-annotation-id='popup_26R'] .popup";
1096+
await page.waitForSelector(popupSelector, { visible: true });
1097+
1098+
await switchToFreeText(page);
1099+
1100+
const editorSelector = getEditorSelector(0);
1101+
await selectEditor(page, editorSelector);
1102+
await page.keyboard.press("Backspace");
1103+
await page.waitForFunction(
1104+
sel => !document.querySelector(sel),
1105+
{},
1106+
editorSelector
1107+
);
1108+
1109+
await waitForSerialized(page, 1);
1110+
const serialized = await getSerialized(page);
1111+
expect(serialized).toEqual([
1112+
{
1113+
pageIndex: 0,
1114+
id: "26R",
1115+
deleted: true,
1116+
popupRef: "",
1117+
},
1118+
]);
1119+
1120+
// Disable editing mode.
1121+
await switchToFreeText(page, /* disable = */ true);
1122+
1123+
await page.waitForSelector(":not([data-annotation-id='26R'] .popup)");
1124+
1125+
// Re-enable editing mode.
1126+
await switchToFreeText(page);
1127+
await page.focus(".annotationEditorLayer");
1128+
1129+
await kbUndo(page);
1130+
await waitForSerialized(page, 0);
1131+
1132+
// Disable editing mode.
1133+
await switchToFreeText(page, /* disable = */ true);
1134+
1135+
const popupAreaSelector =
1136+
"[data-annotation-id='26R'].popupTriggerArea";
1137+
await page.waitForSelector(popupAreaSelector, { visible: true });
1138+
await page.click("[data-annotation-id='26R']");
1139+
// Wait for the popup to be displayed.
1140+
await page.waitForSelector(popupSelector, { visible: true });
1141+
})
1142+
);
1143+
});
10891144
});
10901145

10911146
describe("FreeText (copy/paste existing)", () => {

0 commit comments

Comments
 (0)