fix(web): keep task checkboxes controlled so check-all updates render
remark-gfm passes checked as undefined for unchecked task items, so the
Base UI Checkbox mounted uncontrolled and permanently ignored the
checked=true arriving after "Check all tasks" rewrote the memo content.
Coerce the prop to a boolean so the checkbox is always controlled.
Also drop the `${updateTime}` suffixes from memo React keys and the
comment editor cache key: a protobuf-es Timestamp stringifies to
"[object Object]", so the suffix was a constant — the keys were
effectively name-only already, and now say so honestly.
Fixes usememos/memos#6143
This commit is contained in:
parent
4eee9a5fb4
commit
5192732cee
5 changed files with 71 additions and 7 deletions
|
|
@ -102,7 +102,7 @@ const MemoCommentSection = ({ memo, comments, parentPage, hasMoreComments, isFet
|
|||
{showEditor && EditorComponent && (
|
||||
<div className="w-full mb-2">
|
||||
<EditorComponent
|
||||
cacheKey={`${memo.name}-${memo.updateTime}-comment`}
|
||||
cacheKey={`${memo.name}-comment`}
|
||||
placeholder={t("editor.add-your-comment-here")}
|
||||
parentMemoName={memo.name}
|
||||
autoFocus
|
||||
|
|
@ -112,7 +112,7 @@ const MemoCommentSection = ({ memo, comments, parentPage, hasMoreComments, isFet
|
|||
</div>
|
||||
)}
|
||||
{comments.map((comment) => (
|
||||
<div className="w-full" key={`${comment.name}-${comment.updateTime}`} id={extractMemoIdFromName(comment.name)}>
|
||||
<div className="w-full" key={comment.name} id={extractMemoIdFromName(comment.name)}>
|
||||
<MemoView memo={comment} parentPage={parentPage} showCreator compact />
|
||||
</div>
|
||||
))}
|
||||
|
|
|
|||
|
|
@ -65,6 +65,10 @@ export const TaskListItem: React.FC<TaskListItemProps> = ({ checked, node: _node
|
|||
});
|
||||
};
|
||||
|
||||
// Override the disabled prop from remark-gfm (which defaults to true)
|
||||
return <Checkbox ref={checkboxRef} checked={checked} disabled={readonly} onCheckedChange={handleChange} className={props.className} />;
|
||||
// Override the disabled prop from remark-gfm (which defaults to true).
|
||||
// `checked` is undefined for unchecked tasks; Base UI locks controlled/uncontrolled
|
||||
// mode at mount, so coerce to boolean to keep the checkbox always controlled.
|
||||
return (
|
||||
<Checkbox ref={checkboxRef} checked={Boolean(checked)} disabled={readonly} onCheckedChange={handleChange} className={props.className} />
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -20,8 +20,9 @@ import Placeholder from "../Placeholder";
|
|||
import { estimateMemoCardHeight } from "./memoCardHeight";
|
||||
|
||||
// Memo identity for React keys and grid planning. The pages use it for their renderer keys too,
|
||||
// so flow-list and grid identity can never drift apart.
|
||||
export const getMemoKey = (memo: Memo) => `${memo.name}-${memo.updateTime}`;
|
||||
// so flow-list and grid identity can never drift apart. Deliberately name-only: content updates
|
||||
// reconcile in place (updateTime is a protobuf Timestamp object, not usable in a template string).
|
||||
export const getMemoKey = (memo: Memo) => memo.name;
|
||||
|
||||
// Columns never stretch past this, so 2 columns on a wide monitor stay readable and the
|
||||
// grid centers in the leftover space instead of filling it.
|
||||
|
|
|
|||
|
|
@ -135,7 +135,7 @@ const MemoDetail = () => {
|
|||
</div>
|
||||
)}
|
||||
<MemoView
|
||||
key={`${displayMemo.name}-${displayMemo.updateTime}`}
|
||||
key={displayMemo.name}
|
||||
memo={displayMemo}
|
||||
compact={false}
|
||||
parentPage={parentPage}
|
||||
|
|
|
|||
59
web/tests/task-list-item-check-all.test.tsx
Normal file
59
web/tests/task-list-item-check-all.test.tsx
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
import { render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { TaskListItem } from "@/components/MemoContent/TaskListItem";
|
||||
|
||||
// Regression test for usememos/memos#6143 ("Check all tasks not working").
|
||||
//
|
||||
// remark-gfm passes `checked: undefined` for unchecked task items (`- [ ]`).
|
||||
// Base UI's Checkbox locks controlled/uncontrolled mode at mount, so a checkbox
|
||||
// that mounts with `checked === undefined` becomes uncontrolled and silently
|
||||
// ignores `checked=true` arriving later — exactly what happens after
|
||||
// "Check all tasks" rewrites the memo content and the markdown re-renders.
|
||||
// TaskListItem must coerce `checked` to a boolean so the checkbox is always
|
||||
// controlled.
|
||||
|
||||
const mockUpdateMemo = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@/hooks/useMemoQueries", () => ({
|
||||
useUpdateMemo: () => ({ mutate: mockUpdateMemo }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/MemoView/MemoViewContext", () => ({
|
||||
useMemoViewContext: () => ({
|
||||
memo: {
|
||||
name: "memos/1",
|
||||
content: "- [ ] task one",
|
||||
relations: [],
|
||||
attachments: [],
|
||||
reactions: [],
|
||||
},
|
||||
}),
|
||||
useMemoViewDerived: () => ({
|
||||
readonly: false,
|
||||
}),
|
||||
}));
|
||||
|
||||
describe("<TaskListItem /> check-all regression (#6143)", () => {
|
||||
it("reflects checked=true arriving after mounting unchecked (Check all tasks)", () => {
|
||||
// remark-gfm renders `- [ ]` with `checked` undefined.
|
||||
const { rerender } = render(<TaskListItem checked={undefined} node={undefined} />);
|
||||
|
||||
const checkbox = screen.getByRole("checkbox");
|
||||
expect(checkbox).toHaveAttribute("aria-checked", "false");
|
||||
|
||||
// "Check all tasks" updates memo content; the markdown re-renders with checked=true.
|
||||
rerender(<TaskListItem checked={true} node={undefined} />);
|
||||
expect(checkbox).toHaveAttribute("aria-checked", "true");
|
||||
});
|
||||
|
||||
it("reflects checked reverting to undefined (Uncheck all tasks)", () => {
|
||||
const { rerender } = render(<TaskListItem checked={true} node={undefined} />);
|
||||
|
||||
const checkbox = screen.getByRole("checkbox");
|
||||
expect(checkbox).toHaveAttribute("aria-checked", "true");
|
||||
|
||||
// remark-gfm passes undefined (not false) for `- [ ]` items.
|
||||
rerender(<TaskListItem checked={undefined} node={undefined} />);
|
||||
expect(checkbox).toHaveAttribute("aria-checked", "false");
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue