notes: persist chosen color (and labels) when composer auto-saves

When creating a new note, picking a color from the palette had no
effect — the note saved with the default white background. Editing an
existing note worked because that path lives in a Dialog with no
auto-save guard.

Root cause
The inline Composer in artifacts/tx-os/src/pages/notes.tsx attaches a
document-level mousedown listener that auto-saves whenever the click
falls outside its container ref. The color and label menus are built
on Radix Popover, which renders content in a portal outside the
composer's DOM tree. So clicking a swatch fired:
  1. document mousedown -> save() with the OLD color
  2. swatch click       -> setColor(new) (state already discarded)

Fix
The mousedown handler now ignores clicks whose target is inside any
Radix popper wrapper, Radix portal, or role="dialog" element. The
guard is narrow: legitimate outside clicks (page background, sidebar,
other notes) still trigger save() exactly as before. The same fix
covers the LabelMenu popover for free since it shares the abstraction.

Tests
- New tests/notes-composer-color.spec.mjs reproduces the original
  failure mode: open composer, type title, pick the "red" swatch from
  the popover, click outside, assert the POST /api/notes response has
  color === "red".
- Existing notes-folders + new spec run green together (3 passed in
  29.1s); tx-os tsc --noEmit clean.

Architect notes (followed inline / deferred)
- Suggested mirroring a labels-popover e2e — same code path is already
  exercised; can be added later if regressions surface.
- composedPath() hardening — current target.closest() works for all
  real Radix portals; not needed.
This commit is contained in:
riyadhafraa
2026-05-06 09:59:10 +00:00
parent e96d709bb1
commit d4e0dfc6e5
2 changed files with 125 additions and 2 deletions
+15 -2
View File
@@ -1537,9 +1537,22 @@ function Composer({ labels }: { labels: NoteLabel[] }) {
useEffect(() => {
if (!open) return;
const onClick = (e: MouseEvent) => {
if (!ref.current?.contains(e.target as Node)) {
save();
const target = e.target as Node | null;
if (ref.current?.contains(target)) return;
// Ignore clicks inside Radix popover/portal content (the color and
// label menus render in a portal outside the composer's DOM tree).
// Without this guard, picking a swatch would trigger save() on the
// mousedown — before the swatch's onClick updated `color` — so the
// new color was never persisted.
const el = target instanceof Element ? target : null;
if (
el?.closest(
"[data-radix-popper-content-wrapper],[data-radix-portal],[role='dialog']",
)
) {
return;
}
save();
};
document.addEventListener("mousedown", onClick);
return () => document.removeEventListener("mousedown", onClick);
@@ -0,0 +1,110 @@
// E2E regression for the inline notes composer color picker.
// The composer auto-saves on outside mousedown. Radix popovers render in
// a portal outside the composer's DOM, so picking a color used to count
// as an "outside click" and saved the note with the OLD color before the
// swatch's onClick had a chance to update state. This test guards that
// fix: pick a non-default color, click outside, then assert the saved
// note has that color.
import { test, expect } from "@playwright/test";
import pg from "pg";
const DATABASE_URL = process.env.DATABASE_URL;
if (!DATABASE_URL) {
throw new Error("DATABASE_URL must be set to run this UI test");
}
const TEST_PASSWORD = "TestPass123!";
const TEST_PASSWORD_HASH =
"$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu";
const pool = new pg.Pool({ connectionString: DATABASE_URL });
const createdUserIds = [];
function uniq() {
return `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
}
async function createUser(prefix, displayEn) {
const username = `${prefix}_${uniq()}`;
const { rows } = await pool.query(
`INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
VALUES ($1, $2, $3, $4, 'en', true) RETURNING id`,
[username, `${username}@example.com`, TEST_PASSWORD_HASH, displayEn],
);
const id = rows[0].id;
createdUserIds.push(id);
await pool.query(
`INSERT INTO user_roles (user_id, role_id)
SELECT $1, id FROM roles WHERE name = 'user'`,
[id],
);
return { id, username };
}
test.afterAll(async () => {
if (createdUserIds.length > 0) {
await pool.query(`DELETE FROM notes WHERE user_id = ANY($1::int[])`, [
createdUserIds,
]);
await pool.query(`DELETE FROM user_roles WHERE user_id = ANY($1::int[])`, [
createdUserIds,
]);
await pool.query(`DELETE FROM users WHERE id = ANY($1::int[])`, [
createdUserIds,
]);
}
await pool.end();
});
async function loginViaUi(page, username) {
await page.goto("/login");
await page.locator("#username").fill(username);
await page.locator("#password").fill(TEST_PASSWORD);
await Promise.all([
page.waitForURL((url) => !url.pathname.endsWith("/login"), {
timeout: 15_000,
}),
page.locator('form button[type="submit"]').click(),
]);
}
test("composer persists chosen color when saving via outside click", async ({
page,
}) => {
const user = await createUser("notes_color_e2e", "Color Tester");
await loginViaUi(page, user.username);
await page.goto("/notes");
await expect(page.getByTestId("notes-page")).toBeVisible();
// Open the composer and type a title.
const title = `color-test ${uniq()}`;
await page.getByTestId("notes-composer").click();
await page.getByTestId("notes-composer-title").fill(title);
// Open the color popover and pick a non-default swatch.
await page
.getByTestId("notes-composer")
.locator('button[aria-label="Color"]')
.click();
// NOTE_COLORS[1] is the first non-default swatch ("red"); use it as the
// target. The swatch button has aria-label set to its color id.
const swatch = page.locator('[role="dialog"] button[aria-label="red"]')
.or(page.locator('button[aria-label="red"]'));
await swatch.first().click();
// Click far outside the composer to trigger auto-save.
const savePromise = page.waitForResponse(
(r) =>
new URL(r.url()).pathname === "/api/notes" &&
r.request().method() === "POST" &&
r.status() >= 200 &&
r.status() < 300,
);
await page.mouse.click(10, 10);
const resp = await savePromise;
const note = await resp.json();
expect(note.title).toBe(title);
expect(note.color).toBe("red");
});