Copilot commented on code in PR #7355:
URL: https://github.com/apache/texera/pull/7355#discussion_r3727099481
##########
frontend/src/app/dashboard/component/user/markdown-description/markdown-description.component.spec.ts:
##########
@@ -219,6 +219,81 @@ describe("MarkdownDescriptionComponent", () => {
expect(component.currentMode).toBe("preview");
});
+ /**
+ * The editor toolbar (bold, link, and so on) all routes through `insert`,
which was untested.
Review Comment:
Grammar in the added doc comment: "all routes" should be "all route" (plural
subject "actions").
##########
frontend/src/app/dashboard/component/user/markdown-description/markdown-description.component.spec.ts:
##########
@@ -219,6 +219,81 @@ describe("MarkdownDescriptionComponent", () => {
expect(component.currentMode).toBe("preview");
});
+ /**
+ * The editor toolbar (bold, link, and so on) all routes through `insert`,
which was untested.
+ * It wraps whatever the user has selected, or a placeholder when nothing
is, and must splice the
+ * result back without disturbing the text either side - get the offsets
wrong and the toolbar
+ * silently corrupts the description it is meant to format.
+ *
+ * The tests drive the real textarea from the template, setting
selectionStart/selectionEnd the
+ * way a user's selection would, rather than stubbing the ref.
+ */
+ describe("insert", () => {
+ const bold = { prefix: "**", suffix: "**", default: "bold text" };
+
+ async function editorWith(
+ content: string,
+ selection: [number, number]
+ ): Promise<ComponentFixture<MarkdownDescriptionComponent>> {
+ const fixture = await createFixture();
+ const component = fixture.componentInstance;
+ component.editable = true;
+ // First cycle runs ngOnInit, which forces preview mode; only then can
edit mode be set and
+ // the textarea rendered by a second cycle. Setting it beforehand is
silently overwritten.
+ fixture.detectChanges();
+ component.currentMode = "edit";
+ component.editingContent = content;
+ fixture.detectChanges();
+ const textarea = component.textareaRef.nativeElement;
+ textarea.value = content;
+ textarea.setSelectionRange(selection[0], selection[1]);
+ return fixture;
+ }
+
+ it("wraps the selected text and leaves the surrounding text intact", async
() => {
+ // Select "world" out of "hello world!" - the leading "hello " and
trailing "!" must survive.
+ const fixture = await editorWith("hello world!", [6, 11]);
+
+ fixture.componentInstance.insert(bold);
+
+ expect(fixture.componentInstance.editingContent).toBe("hello
**world**!");
+ });
+
+ it("inserts the placeholder when nothing is selected", async () => {
+ // A collapsed caret means there is nothing to wrap, so the action's
default stands in and
+ // the user can type over it.
+ const fixture = await editorWith("hello ", [6, 6]);
+
+ fixture.componentInstance.insert(bold);
+
+ expect(fixture.componentInstance.editingContent).toBe("hello **bold
text**");
+ });
+
+ it("uses the action's own prefix and suffix rather than a fixed pair",
async () => {
+ // A link action is asymmetric, which a hardcoded "wrap in prefix twice"
would get wrong.
+ const fixture = await editorWith("see docs", [4, 8]);
+
+ fixture.componentInstance.insert({ prefix: "[", suffix: "](url)",
default: "text" });
+
+ expect(fixture.componentInstance.editingContent).toBe("see [docs](url)");
+ });
+
+ it("re-renders the preview from the spliced content", async () => {
+ const fixture = await editorWith("hi", [0, 2]);
+ parse.mockClear();
+
+ fixture.componentInstance.insert(bold);
+ // Not whenStable(): insert() schedules a requestAnimationFrame to
refocus the textarea, which
+ // keeps the zone permanently unstable and hangs the test.
renderMarkdown resolves through a
+ // promise, so draining the microtask queue is both sufficient and
terminating.
+ await new Promise(resolve => setTimeout(resolve, 0));
Review Comment:
The test says it is draining the microtask queue, but it awaits a
setTimeout(0) macrotask. This adds real-time delay and is less precise than
awaiting a resolved Promise (renderMarkdown uses
Promise.resolve(...).then(...)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
