Skip to content

Commit 3432c19

Browse files
authored
Merge pull request #20249 from calixteman/new_comment_popup_2
[Editor] Add a new popup for comments (bug 1987425)
2 parents 0982ff2 + b660b72 commit 3432c19

19 files changed

+1334
-700
lines changed

extensions/chromium/preferences_schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@
7171
"type": "string",
7272
"default": ""
7373
},
74+
"commentLearnMoreUrl": {
75+
"type": "string",
76+
"default": ""
77+
},
7478
"enableSignatureEditor": {
7579
"type": "boolean",
7680
"default": false

l10n/en-US/viewer.ftl

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,8 @@ pdfjs-editor-comments-sidebar-close-button =
414414
pdfjs-editor-comments-sidebar-close-button-label = Close the sidebar
415415
416416
# Instructional copy to add a comment by selecting text or an annotations.
417-
pdfjs-editor-comments-sidebar-no-comments = Add a comment by selecting text or an annotation.
417+
pdfjs-editor-comments-sidebar-no-comments1 = See something noteworthy? Highlight it and leave a comment.
418+
pdfjs-editor-comments-sidebar-no-comments-link = Learn more
418419
419420
## Alt-text dialog
420421

@@ -670,21 +671,28 @@ pdfjs-editor-edit-signature-dialog-title = Edit description
670671

671672
pdfjs-editor-edit-signature-update-button = Update
672673
674+
## Comment popup
675+
676+
pdfjs-editor-edit-comment-popup-button-label = Edit comment
677+
pdfjs-editor-edit-comment-popup-button =
678+
.title = Edit comment
679+
pdfjs-editor-delete-comment-popup-button-label = Remove comment
680+
pdfjs-editor-delete-comment-popup-button =
681+
.title = Remove comment
682+
673683
## Edit a comment dialog
674684

675-
pdfjs-editor-edit-comment-actions-button-label = Actions
676-
pdfjs-editor-edit-comment-actions-button =
677-
.title = Actions
678-
pdfjs-editor-edit-comment-close-button-label = Close
679-
pdfjs-editor-edit-comment-close-button =
680-
.title = Close
681-
pdfjs-editor-edit-comment-actions-edit-button-label = Edit
682-
pdfjs-editor-edit-comment-actions-delete-button-label = Delete
683-
pdfjs-editor-edit-comment-manager-text-input =
684-
.placeholder = Enter your comment
685+
# An existing comment is edited
686+
pdfjs-editor-edit-comment-dialog-title-when-editing = Edit comment
687+
688+
# No existing comment
689+
pdfjs-editor-edit-comment-dialog-title-when-adding = Add comment
690+
691+
pdfjs-editor-edit-comment-dialog-text-input =
692+
.placeholder = Start typing…
685693
686-
pdfjs-editor-edit-comment-manager-cancel-button = Cancel
687-
pdfjs-editor-edit-comment-manager-save-button = Save
694+
pdfjs-editor-edit-comment-dialog-cancel-button = Cancel
695+
pdfjs-editor-edit-comment-dialog-save-button = Save
688696
689697
## Edit a comment button in the editor toolbar
690698

src/display/editor/annotation_editor_layer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ class AnnotationEditorLayer {
334334
if (editor?.annotationElementId === null) {
335335
e.stopPropagation();
336336
e.preventDefault();
337-
editor.dblclick();
337+
editor.dblclick(e);
338338
}
339339
},
340340
{ signal, capture: true }

src/display/editor/comment.js

Lines changed: 147 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* limitations under the License.
1414
*/
1515

16-
import { noContextMenu } from "../display_utils.js";
16+
import { noContextMenu, stopEvent } from "../display_utils.js";
1717

1818
class Comment {
1919
#commentStandaloneButton = null;
@@ -34,6 +34,8 @@ class Comment {
3434

3535
#deleted = false;
3636

37+
#popupPosition = null;
38+
3739
constructor(editor) {
3840
this.#editor = editor;
3941
}
@@ -42,7 +44,7 @@ class Comment {
4244
const button = (this.#commentToolbarButton =
4345
document.createElement("button"));
4446
button.className = "comment";
45-
return this.#render(button);
47+
return this.#render(button, false);
4648
}
4749

4850
renderForStandalone() {
@@ -66,23 +68,118 @@ class Comment {
6668
}
6769
}
6870

69-
return this.#render(button);
71+
return this.#render(button, true);
72+
}
73+
74+
onUpdatedColor() {
75+
if (!this.#commentStandaloneButton) {
76+
return;
77+
}
78+
const color = this.#editor.commentButtonColor;
79+
if (color) {
80+
this.#commentStandaloneButton.style.backgroundColor = color;
81+
}
82+
this.#editor._uiManager.updatePopupColor(this.#editor);
83+
}
84+
85+
get commentButtonWidth() {
86+
return (
87+
(this.#commentStandaloneButton?.getBoundingClientRect().width ?? 0) /
88+
this.#editor.parent.boundingClientRect.width
89+
);
7090
}
7191

72-
#render(comment) {
92+
get commentPopupPositionInLayer() {
93+
if (this.#popupPosition) {
94+
return this.#popupPosition;
95+
}
96+
if (!this.#commentStandaloneButton) {
97+
return null;
98+
}
99+
const { x, y, height } =
100+
this.#commentStandaloneButton.getBoundingClientRect();
101+
const {
102+
x: parentX,
103+
y: parentY,
104+
width: parentWidth,
105+
height: parentHeight,
106+
} = this.#editor.parent.boundingClientRect;
107+
const OFFSET_UNDER_BUTTON = 2;
108+
return [
109+
(x - parentX) / parentWidth,
110+
(y + height + OFFSET_UNDER_BUTTON - parentY) / parentHeight,
111+
];
112+
}
113+
114+
set commentPopupPositionInLayer(pos) {
115+
this.#popupPosition = pos;
116+
}
117+
118+
removeStandaloneCommentButton() {
119+
this.#commentStandaloneButton?.remove();
120+
this.#commentStandaloneButton = null;
121+
}
122+
123+
removeToolbarCommentButton() {
124+
this.#commentToolbarButton?.remove();
125+
this.#commentToolbarButton = null;
126+
}
127+
128+
setCommentButtonStates({ selected, hasPopup }) {
129+
if (!this.#commentStandaloneButton) {
130+
return;
131+
}
132+
this.#commentStandaloneButton.classList.toggle("selected", selected);
133+
this.#commentStandaloneButton.ariaExpanded = hasPopup;
134+
}
135+
136+
#render(comment, isStandalone) {
73137
if (!this.#editor._uiManager.hasCommentManager()) {
74138
return null;
75139
}
76140

77141
comment.tabIndex = "0";
78-
comment.setAttribute("data-l10n-id", "pdfjs-editor-edit-comment-button");
142+
comment.ariaHasPopup = "dialog";
143+
144+
if (isStandalone) {
145+
comment.ariaControls = "commentPopup";
146+
} else {
147+
comment.ariaControlsElements = [
148+
this.#editor._uiManager.getCommentDialogElement(),
149+
];
150+
comment.setAttribute("data-l10n-id", "pdfjs-editor-edit-comment-button");
151+
}
79152

80153
const signal = this.#editor._uiManager._signal;
81154
if (!(signal instanceof AbortSignal) || signal.aborted) {
82155
return comment;
83156
}
84157

85158
comment.addEventListener("contextmenu", noContextMenu, { signal });
159+
if (isStandalone) {
160+
comment.addEventListener(
161+
"focusin",
162+
e => {
163+
this.#editor._focusEventsAllowed = false;
164+
stopEvent(e);
165+
},
166+
{
167+
capture: true,
168+
signal,
169+
}
170+
);
171+
comment.addEventListener(
172+
"focusout",
173+
e => {
174+
this.#editor._focusEventsAllowed = true;
175+
stopEvent(e);
176+
},
177+
{
178+
capture: true,
179+
signal,
180+
}
181+
);
182+
}
86183
comment.addEventListener("pointerdown", event => event.stopPropagation(), {
87184
signal,
88185
});
@@ -92,7 +189,7 @@ class Comment {
92189
if (comment === this.#commentToolbarButton) {
93190
this.edit();
94191
} else {
95-
this.#editor._uiManager.toggleComment(this.#editor);
192+
this.#editor.toggleComment(/* isSelected = */ true);
96193
}
97194
};
98195
comment.addEventListener("click", onClick, { capture: true, signal });
@@ -107,18 +204,55 @@ class Comment {
107204
{ signal }
108205
);
109206

207+
comment.addEventListener(
208+
"pointerenter",
209+
() => {
210+
this.#editor.toggleComment(
211+
/* isSelected = */ false,
212+
/* visibility = */ true
213+
);
214+
},
215+
{ signal }
216+
);
217+
comment.addEventListener(
218+
"pointerleave",
219+
() => {
220+
this.#editor.toggleComment(
221+
/* isSelected = */ false,
222+
/* visibility = */ false
223+
);
224+
},
225+
{ signal }
226+
);
227+
110228
return comment;
111229
}
112230

113-
edit() {
114-
const { bottom, left, right } = this.#editor.getClientDimensions();
115-
const position = { top: bottom };
116-
if (this.#editor._uiManager.direction === "ltr") {
117-
position.right = right;
231+
edit(options) {
232+
const position = this.commentPopupPositionInLayer;
233+
let posX, posY;
234+
if (position) {
235+
[posX, posY] = position;
118236
} else {
119-
position.left = left;
237+
// The position is in the editor coordinates.
238+
[posX, posY] = this.#editor.commentButtonPosition;
239+
const { width, height, x, y } = this.#editor;
240+
posX = x + posX * width;
241+
posY = y + posY * height;
120242
}
121-
this.#editor._uiManager.editComment(this.#editor, position);
243+
const parentDimensions = this.#editor.parent.boundingClientRect;
244+
const {
245+
x: parentX,
246+
y: parentY,
247+
width: parentWidth,
248+
height: parentHeight,
249+
} = parentDimensions;
250+
this.#editor._uiManager.editComment(
251+
this.#editor,
252+
parentX + posX * parentWidth,
253+
parentY + posY * parentHeight,
254+
{ ...options, parentDimensions }
255+
);
122256
}
123257

124258
finish() {

src/display/editor/draw.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ class DrawingEditor extends AnnotationEditor {
9898
this._addOutlines(params);
9999
}
100100

101+
/** @inheritdoc */
102+
onUpdatedColor() {
103+
this._colorPicker?.update(this.color);
104+
super.onUpdatedColor();
105+
}
106+
101107
_addOutlines(params) {
102108
if (params.drawOutlines) {
103109
this.#createDrawOutlines(params);
@@ -243,7 +249,7 @@ class DrawingEditor extends AnnotationEditor {
243249
options.toSVGProperties()
244250
);
245251
if (type === this.colorType) {
246-
this._colorPicker?.update(val);
252+
this.onUpdatedColor();
247253
}
248254
};
249255
this.addCommands({

0 commit comments

Comments
 (0)