From 1c26ad80c918a49cf11e98649c2bd440342b26d6 Mon Sep 17 00:00:00 2001 From: OdontoDev <313455402+odontodev@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:45:59 -0600 Subject: [PATCH 1/3] fix: make every session-setting setter idempotent and notify onStateChange setNotesEnabled, setIcdasEnabled, setCariesDepthEnabled, setSecondaryCariesMode, setRootCariesMode, setRadiographicDepthMode, setWearDetailLevel, setDiscolorationDetailLevel and setNumberingSystem now follow the convention setPulpDetailLevel/setSurfaceNotation and the fillings setters already use: early-return when unchanged, notifyStateChange() on a real change. A host that persists the doctor's preferences through onStateChange otherwise never sees a toggle made in the Settings modal until the next tooth edit. Co-Authored-By: Claude Opus 5 --- .../session-settings-setters.test.ts | 114 ++++++++++++++++++ src/__tests__/tooth-details-card.test.tsx | 14 +-- src/odontogram.ts | 44 +++++-- src/state/notify.ts | 5 +- 4 files changed, 161 insertions(+), 16 deletions(-) create mode 100644 src/__tests__/session-settings-setters.test.ts diff --git a/src/__tests__/session-settings-setters.test.ts b/src/__tests__/session-settings-setters.test.ts new file mode 100644 index 00000000..a9c646e2 --- /dev/null +++ b/src/__tests__/session-settings-setters.test.ts @@ -0,0 +1,114 @@ +// Part of React Advanced Odontogram - https://github.com/ZoliQua/React-Advanced-Odontogram +// Created by Zoltan Dul (https://github.com/ZoliQua) 2025-2026 + +// The remaining session-setting setters must follow the same convention as +// setPulpDetailLevel/setSurfaceNotation and the fillings setters +// (filling-settings-setters.test.ts): idempotent (early-return when the value +// is unchanged) AND call notifyStateChange(), so a host that persists the +// doctor's preferences through onStateChange sees a toggle made in the +// settings modal even when no tooth is edited afterwards. +import { describe, it, expect, afterEach } from "vitest"; +import { + setNotesEnabled, + getNotesEnabled, + setIcdasEnabled, + getIcdasEnabled, + setCariesDepthEnabled, + getCariesDepthEnabled, + setSecondaryCariesMode, + getSecondaryCariesMode, + setRootCariesMode, + getRootCariesMode, + setRadiographicDepthMode, + getRadiographicDepthMode, + setWearDetailLevel, + getWearDetailLevel, + setDiscolorationDetailLevel, + getDiscolorationDetailLevel, + setNumberingSystem, + onStateChange, +} from "../odontogram"; +import { numberingSystem } from "../state/numbering"; + +const resetDefaults = () => { + setNotesEnabled(false); + setIcdasEnabled(false); + setCariesDepthEnabled(true); + setSecondaryCariesMode("standard"); + setRootCariesMode("simple"); + setRadiographicDepthMode("off"); + setWearDetailLevel("complex"); + setDiscolorationDetailLevel("complex"); + setNumberingSystem("FDI"); +}; + +/** Runs `fn` with a subscriber attached and reports whether it fired. */ +function fires(fn: () => void): boolean { + let fired = false; + const unsub = onStateChange(() => { fired = true; }); + try { fn(); } finally { unsub(); } + return fired; +} + +describe("session-setting setters: idempotent + notifyStateChange", () => { + afterEach(resetDefaults); + + it("setNotesEnabled", () => { + expect(fires(() => setNotesEnabled(true))).toBe(true); + expect(getNotesEnabled()).toBe(true); + expect(fires(() => setNotesEnabled(true))).toBe(false); + // Truthy coercion: a truthy non-boolean is sanitized to true. + expect(fires(() => setNotesEnabled(1 as unknown as boolean))).toBe(false); + }); + + it("setIcdasEnabled", () => { + expect(fires(() => setIcdasEnabled(true))).toBe(true); + expect(getIcdasEnabled()).toBe(true); + expect(fires(() => setIcdasEnabled(true))).toBe(false); + }); + + it("setCariesDepthEnabled", () => { + expect(fires(() => setCariesDepthEnabled(false))).toBe(true); + expect(getCariesDepthEnabled()).toBe(false); + expect(fires(() => setCariesDepthEnabled(false))).toBe(false); + }); + + it("setSecondaryCariesMode", () => { + expect(fires(() => setSecondaryCariesMode("full"))).toBe(true); + expect(getSecondaryCariesMode()).toBe("full"); + expect(fires(() => setSecondaryCariesMode("full"))).toBe(false); + // An invalid value sanitizes to "standard" — a real change from "full". + expect(fires(() => setSecondaryCariesMode("bogus" as "simple"))).toBe(true); + expect(getSecondaryCariesMode()).toBe("standard"); + }); + + it("setRootCariesMode", () => { + expect(fires(() => setRootCariesMode("severity"))).toBe(true); + expect(getRootCariesMode()).toBe("severity"); + expect(fires(() => setRootCariesMode("severity"))).toBe(false); + }); + + it("setRadiographicDepthMode", () => { + expect(fires(() => setRadiographicDepthMode("detailed"))).toBe(true); + expect(getRadiographicDepthMode()).toBe("detailed"); + expect(fires(() => setRadiographicDepthMode("detailed"))).toBe(false); + }); + + it("setWearDetailLevel", () => { + expect(fires(() => setWearDetailLevel("simple"))).toBe(true); + expect(getWearDetailLevel()).toBe("simple"); + expect(fires(() => setWearDetailLevel("simple"))).toBe(false); + }); + + it("setDiscolorationDetailLevel", () => { + expect(fires(() => setDiscolorationDetailLevel("simple"))).toBe(true); + expect(getDiscolorationDetailLevel()).toBe("simple"); + expect(fires(() => setDiscolorationDetailLevel("simple"))).toBe(false); + }); + + it("setNumberingSystem", () => { + expect(fires(() => setNumberingSystem("UNIVERSAL"))).toBe(true); + expect(numberingSystem).toBe("UNIVERSAL"); + expect(fires(() => setNumberingSystem("UNIVERSAL"))).toBe(false); + }); +}); diff --git a/src/__tests__/tooth-details-card.test.tsx b/src/__tests__/tooth-details-card.test.tsx index 9474c92a..656a6e73 100644 --- a/src/__tests__/tooth-details-card.test.tsx +++ b/src/__tests__/tooth-details-card.test.tsx @@ -13,7 +13,7 @@ // predicates, that `#extractionPlanRow` reparents to the correct container per // state, and that the title `#btnResetTooth` resets the active tooth. import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { createElement } from "react"; +import { createElement, type ComponentProps } from "react"; import { render, cleanup, fireEvent } from "@testing-library/react"; import { OdontogramProvider, ToothControlsSurface } from "../App"; import { @@ -37,8 +37,8 @@ vi.mock("../odontogram", async (importOriginal) => { }; }); -function renderControls() { - return render(createElement(OdontogramProvider, { language: "en" }, createElement(ToothControlsSurface))); +function renderControls(props: Partial> = {}) { + return render(createElement(OdontogramProvider, { language: "en", ...props }, createElement(ToothControlsSurface))); } beforeEach(() => { @@ -116,13 +116,13 @@ describe("PR 3f: renders declaratively", () => { expect(document.getElementById("discolorationToggleLabel")?.classList.contains("hidden")).toBe(true); // Simple: the toggles are shown, the selects hidden (level flips them, never - // mutating the stored value). + // mutating the stored value). The level goes in as a provider prop: the + // provider's mount effect pushes its props into the module, and since the + // setters now notify, that push re-renders the card. cleanup(); - setWearDetailLevel("simple"); - setDiscolorationDetailLevel("simple"); __setToothStateForTest(11, { toothSelection: "tooth-base" }); __setSelectionForTest([11]); - renderControls(); + renderControls({ wearDetailLevel: "simple", discolorationDetailLevel: "simple" }); expect(document.getElementById("wearEdgeSelectLabel")?.classList.contains("hidden")).toBe(true); expect(document.getElementById("wearEdgeToggleLabel")?.classList.contains("hidden")).toBe(false); expect(document.getElementById("discolorationSelectLabel")?.classList.contains("hidden")).toBe(true); diff --git a/src/odontogram.ts b/src/odontogram.ts index f1361bfb..a23c4088 100644 --- a/src/odontogram.ts +++ b/src/odontogram.ts @@ -564,7 +564,13 @@ export function toggleCollapsedCard(id: string): void { notifyStateChange(); } let icdasEnabled = false; -export function setIcdasEnabled(value: boolean){ icdasEnabled = !!value; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); } +export function setIcdasEnabled(value: boolean){ + const next = !!value; + if(next === icdasEnabled) return; + icdasEnabled = next; + if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); +} export function getIcdasEnabled(): boolean { return icdasEnabled; } // The pulp-detail level drives how the pulp control presents the pulp // diagnosis — "simple" (healthy/pulpitis), "aae" (4 AAE pulpDx values, default) @@ -591,14 +597,20 @@ export function setPulpDetailLevel(value: PulpDetailLevel){ export type ToothDetailLevel = "simple" | "complex"; let wearDetailLevel: ToothDetailLevel = "complex"; export function setWearDetailLevel(value: ToothDetailLevel){ - wearDetailLevel = value === "simple" ? "simple" : "complex"; + const next = value === "simple" ? "simple" : "complex"; + if(next === wearDetailLevel) return; + wearDetailLevel = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getWearDetailLevel(): ToothDetailLevel { return wearDetailLevel; } let discolorationDetailLevel: ToothDetailLevel = "complex"; export function setDiscolorationDetailLevel(value: ToothDetailLevel){ - discolorationDetailLevel = value === "simple" ? "simple" : "complex"; + const next = value === "simple" ? "simple" : "complex"; + if(next === discolorationDetailLevel) return; + discolorationDetailLevel = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getDiscolorationDetailLevel(): ToothDetailLevel { return discolorationDetailLevel; } @@ -641,29 +653,41 @@ export type RadiographicDepthMode = "off" | "threeLevel" | "detailed"; let secondaryCariesMode: SecondaryCariesMode = "standard"; export function setSecondaryCariesMode(value: SecondaryCariesMode){ - secondaryCariesMode = (value === "simple" || value === "full") ? value : "standard"; + const next = (value === "simple" || value === "full") ? value : "standard"; + if(next === secondaryCariesMode) return; + secondaryCariesMode = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getSecondaryCariesMode(): SecondaryCariesMode { return secondaryCariesMode; } let rootCariesMode: RootCariesMode = "simple"; export function setRootCariesMode(value: RootCariesMode){ - rootCariesMode = (value === "severity") ? value : "simple"; + const next = (value === "severity") ? value : "simple"; + if(next === rootCariesMode) return; + rootCariesMode = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getRootCariesMode(): RootCariesMode { return rootCariesMode; } let radiographicDepthMode: RadiographicDepthMode = "off"; export function setRadiographicDepthMode(value: RadiographicDepthMode){ - radiographicDepthMode = (value === "threeLevel" || value === "detailed") ? value : "off"; + const next = (value === "threeLevel" || value === "detailed") ? value : "off"; + if(next === radiographicDepthMode) return; + radiographicDepthMode = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getRadiographicDepthMode(): RadiographicDepthMode { return radiographicDepthMode; } let cariesDepthEnabled = true; export function setCariesDepthEnabled(value: boolean){ - cariesDepthEnabled = value !== false; + const next = value !== false; + if(next === cariesDepthEnabled) return; + cariesDepthEnabled = next; if(activeTooth) syncControlsFromState(toothState.get(activeTooth)); + notifyStateChange(); } export function getCariesDepthEnabled(): boolean { return cariesDepthEnabled; } @@ -7819,6 +7843,7 @@ export function setNumberingSystem(system: NumberingSystem){ if(!applyNumberingSystem(system)) return; updateAllToothTileNumbers(); updateActiveLabel(); + notifyStateChange(); } /** @@ -8549,12 +8574,15 @@ export function getReadOnly(): boolean{ * @param value - `true` to enable notes, `false` to disable. */ export function setNotesEnabled(value: boolean){ - notesEnabled = value; + const next = !!value; + if(next === notesEnabled) return; + notesEnabled = next; // Refresh tooltips and label icons for all teeth for(const toothNo of ALL_TEETH){ updateToothTooltip(toothNo); updateToothLabelNoteIcon(toothNo); } + notifyStateChange(); } /** diff --git a/src/state/notify.ts b/src/state/notify.ts index e3d40e4e..f192a934 100644 --- a/src/state/notify.ts +++ b/src/state/notify.ts @@ -25,7 +25,10 @@ export function setPostNotifyHook(fn: (() => void) | null): void { /** * Subscribe to odontogram state changes. The callback runs after any tooth - * state edit, the edentulous toggle, or an import. + * state edit, the edentulous toggle, an import, or a change to a session + * setting (numbering system, notes, ICDAS, caries/pulp/wear/discoloration + * detail, surface notation, fillings, perio display) — so a host that + * persists the doctor's preferences can react to the settings modal too. * * @param cb - Callback invoked on each change. * @returns An unsubscribe function. From a82227f65b1794302a2c48b1d4d2604d183b63b3 Mon Sep 17 00:00:00 2001 From: OdontoDev <313455402+odontodev@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:45:59 -0600 Subject: [PATCH 2/3] fix: notify onStateChange when a tooth note is saved or deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The note editor popover wrote state.note without notifyStateChange(), so the note — part of the export payload — reached neither the localStorage autosave nor a host persisting the chart until the next unrelated edit. Only a real change notifies; re-saving the same text or deleting an empty note is a no-op. Co-Authored-By: Claude Opus 5 --- src/__tests__/note-editor-notify.test.tsx | 114 ++++++++++++++++++++++ src/odontogram.ts | 18 ++-- 2 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/note-editor-notify.test.tsx diff --git a/src/__tests__/note-editor-notify.test.tsx b/src/__tests__/note-editor-notify.test.tsx new file mode 100644 index 00000000..c9c89043 --- /dev/null +++ b/src/__tests__/note-editor-notify.test.tsx @@ -0,0 +1,114 @@ +// Part of React Advanced Odontogram - https://github.com/ZoliQua/React-Advanced-Odontogram +// Created by Zoltan Dul (https://github.com/ZoliQua) 2025-2026 + +// The per-tooth note is part of the export payload, so saving or deleting one +// from the note-editor popover must fire notifyStateChange() like any other +// tooth edit — otherwise the localStorage autosave and hosts persisting the +// chart through onStateChange miss it until the next unrelated edit. +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { createElement } from "react"; +import { render, cleanup, fireEvent, waitFor } from "@testing-library/react"; +import App from "../App"; +import { + __resetChartStateForTest, + getStatusChart, + onStateChange, + setNotesEnabled, + setNumberingSystem, +} from "../odontogram"; + +vi.setConfig({ testTimeout: 30000, hookTimeout: 30000 }); + +// Same jsdom stubs as tier2-rewire.test.tsx: a real initOdontogram() needs them. +function installDomStubs() { + window.matchMedia = ((q: string) => ({ + matches: false, media: q, onchange: null, + addEventListener() {}, removeEventListener() {}, + addListener() {}, removeListener() {}, dispatchEvent() { return false; }, + })) as unknown as typeof window.matchMedia; + if (typeof (globalThis as { ResizeObserver?: unknown }).ResizeObserver === "undefined") { + (globalThis as { ResizeObserver?: unknown }).ResizeObserver = class { + observe() {} unobserve() {} disconnect() {} + }; + } +} + +beforeEach(() => { + installDomStubs(); + cleanup(); + document.body.innerHTML = ""; + __resetChartStateForTest(); + setNumberingSystem("FDI"); +}); + +afterEach(() => { + setNotesEnabled(false); + cleanup(); +}); + +async function waitForGrid() { + await waitFor(() => { + const grid = document.getElementById("toothGrid"); + expect(grid && grid.childElementCount).toBeGreaterThan(0); + }); +} + +function openNoteEditor(toothNo: number): { textarea: HTMLTextAreaElement; save: HTMLButtonElement; del: HTMLButtonElement } { + const tile = document.querySelector(`.tooth-tile.side-view[data-tooth="${toothNo}"]`) as HTMLElement; + fireEvent.dblClick(tile); + const popover = document.querySelector(".odon-note-popover") as HTMLElement; + expect(popover).toBeTruthy(); + const [save, del] = Array.from(popover.querySelectorAll(".odon-note-actions button")); + return { textarea: popover.querySelector("textarea") as HTMLTextAreaElement, save: save!, del: del! }; +} + +function notifiesFor(action: () => void): number { + const spy = vi.fn(); + const unsub = onStateChange(spy); + action(); + unsub(); + return spy.mock.calls.length; +} + +describe("note editor: save/delete notify onStateChange", () => { + it("saving a new note notifies once and lands in the payload", async () => { + render(createElement(App, { language: "en", enableNotes: true })); + await waitForGrid(); + + const { textarea, save } = openNoteEditor(11); + textarea.value = " sensitive to cold "; + expect(notifiesFor(() => fireEvent.click(save))).toBe(1); + expect(getStatusChart().teeth[11].note).toBe("sensitive to cold"); + expect(document.querySelector(".odon-note-popover")).toBeNull(); + }); + + it("re-saving the same note does not notify", async () => { + render(createElement(App, { language: "en", enableNotes: true })); + await waitForGrid(); + + const first = openNoteEditor(11); + first.textarea.value = "same"; + fireEvent.click(first.save); + + const second = openNoteEditor(11); + expect(second.textarea.value).toBe("same"); + expect(notifiesFor(() => fireEvent.click(second.save))).toBe(0); + }); + + it("deleting a note notifies once and removes it from the payload", async () => { + render(createElement(App, { language: "en", enableNotes: true })); + await waitForGrid(); + + const first = openNoteEditor(11); + first.textarea.value = "to be removed"; + fireEvent.click(first.save); + + const second = openNoteEditor(11); + expect(notifiesFor(() => fireEvent.click(second.del))).toBe(1); + expect(getStatusChart().teeth[11].note).toBeUndefined(); + + // Deleting an already-empty note is a no-op. + const third = openNoteEditor(11); + expect(notifiesFor(() => fireEvent.click(third.del))).toBe(0); + }); +}); diff --git a/src/odontogram.ts b/src/odontogram.ts index a23c4088..5e6d5ee3 100644 --- a/src/odontogram.ts +++ b/src/odontogram.ts @@ -5012,19 +5012,19 @@ function showNoteEditor(toothNo: number){ const actions = el("div", { class: "odon-note-actions" }); const saveBtn = el("button", { class: "odon-zoom-btn", text: t("note.save") }); - saveBtn.addEventListener("click", () => { - state.note = textarea.value.trim(); + // The note is part of the export payload, so a real change must notify + // (autosave + hosts persisting the chart) like any other tooth edit. + const commitNote = (next: string) => { + const changed = next !== (state.note || ""); + state.note = next; updateToothTooltip(toothNo); updateToothLabelNoteIcon(toothNo); hideNoteEditor(); - }); + if(changed) notifyStateChange(); + }; + saveBtn.addEventListener("click", () => commitNote(textarea.value.trim())); const deleteBtn = el("button", { class: "odon-zoom-btn danger", text: t("note.delete") }); - deleteBtn.addEventListener("click", () => { - state.note = ""; - updateToothTooltip(toothNo); - updateToothLabelNoteIcon(toothNo); - hideNoteEditor(); - }); + deleteBtn.addEventListener("click", () => commitNote("")); actions.appendChild(saveBtn); actions.appendChild(deleteBtn); From 32e2bc94086923589480aeb8d855633101216cb7 Mon Sep 17 00:00:00 2001 From: OdontoDev <313455402+odontodev@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:45:59 -0600 Subject: [PATCH 3/3] fix: restore tooth note badges when the grid is rebuilt buildGrid() repaints every tooth from state through addTile() but creates the label cells bare, so after a re-init, an anatomy change or a perio round-trip a charted note kept its text and lost its badge. Co-Authored-By: Claude Opus 5 --- src/__tests__/note-editor-notify.test.tsx | 16 ++++++++++++++++ src/odontogram.ts | 7 +++++++ 2 files changed, 23 insertions(+) diff --git a/src/__tests__/note-editor-notify.test.tsx b/src/__tests__/note-editor-notify.test.tsx index c9c89043..7b0c62d1 100644 --- a/src/__tests__/note-editor-notify.test.tsx +++ b/src/__tests__/note-editor-notify.test.tsx @@ -13,6 +13,7 @@ import { __resetChartStateForTest, getStatusChart, onStateChange, + rebuildGrid, setNotesEnabled, setNumberingSystem, } from "../odontogram"; @@ -111,4 +112,19 @@ describe("note editor: save/delete notify onStateChange", () => { const third = openNoteEditor(11); expect(notifiesFor(() => fireEvent.click(third.del))).toBe(0); }); + + it("the label badge survives a grid rebuild", async () => { + render(createElement(App, { language: "en", enableNotes: true })); + await waitForGrid(); + + const { textarea, save } = openNoteEditor(11); + textarea.value = "keep me"; + fireEvent.click(save); + const badge = () => document.querySelectorAll(".tooth-label-cell .tooth-note-icon").length; + expect(badge()).toBe(1); + + await rebuildGrid(); + await waitForGrid(); + expect(badge()).toBe(1); + }); }); diff --git a/src/odontogram.ts b/src/odontogram.ts index 5e6d5ee3..61d4fa99 100644 --- a/src/odontogram.ts +++ b/src/odontogram.ts @@ -7557,6 +7557,13 @@ async function buildGrid(token: number){ addLabelRow(lowerSide, toothLabelLower); } + // addTile() repainted each tooth from state; the label cells are fresh, so + // a rebuild (re-init, anatomy change, perio round-trip) must restore the + // note icons the same way, or a charted note shows up with no badge. + for(const toothNo of ALL_TEETH){ + updateToothLabelNoteIcon(toothNo); + } + // ARIA on grid container grid.setAttribute("role", "listbox"); grid.setAttribute("aria-multiselectable", "true");