fix(datums): tighten primary-flag handling
Some checks failed
Deploy to GitHub Pages / build (push) Has been cancelled
Deploy to GitHub Pages / deploy (push) Has been cancelled

- setAxisRole: gate the "clear other datums' flags" loop on
  role !== null. Previously, calling setAxisRole(id, null) — e.g.
  clicking "None" on an unflagged line button — would silently strip
  the primary off whichever rect/ellipse legitimately held it. The
  docstring's "no-op if it wasn't set" promise now actually holds.
- setAxisRole: replace the bare else in the null branch with an
  explicit `else if (target.type === "ellipse")` so a future datum
  type added to the union can't quietly inherit isPrimary.
- pickPrimary: extract `flaggedPrimary(d)` so the user-flag check
  reads as "is this datum flagged?" rather than implying a check
  order across types. Adds a comment documenting that mutual
  exclusion is enforced by setAxisRole and that bypassing the store
  would fall back to array-order tie-breaking.
This commit is contained in:
Samuel Prevost 2026-04-30 23:52:25 +02:00
parent c2f7bf0df2
commit 23d3297434
2 changed files with 36 additions and 24 deletions

View File

@ -265,23 +265,28 @@ type Primary =
| { kind: "line"; datum: LineDatum } | { kind: "line"; datum: LineDatum }
| { kind: "ellipse"; datum: EllipseDatum } | { kind: "ellipse"; datum: EllipseDatum }
function flaggedPrimary(d: Datum): Primary | null {
if (d.type === "rectangle" && d.isAxisReference) return { kind: "rect", datum: d }
if (d.type === "line" && d.axisRole) return { kind: "line", datum: d }
if (d.type === "ellipse" && d.isPrimary) return { kind: "ellipse", datum: d }
return null
}
function pickPrimary(datums: Datum[]): Primary { function pickPrimary(datums: Datum[]): Primary {
if (datums.length === 0) throw new Error("No datums provided.") if (datums.length === 0) throw new Error("No datums provided.")
// User-flagged primary wins regardless of type priority. Rect's // User-flagged primary wins regardless of type. The store
// `isAxisReference` and line's `axisRole` carry axis semantics on top // (`setAxisRole`) enforces mutual exclusion across the three flag
// of "primary"; ellipse's `isPrimary` is a pure primary flag (ellipses // kinds — `RectDatum.isAxisReference`, `LineDatum.axisRole`, and
// don't define axis directions on their own). // `EllipseDatum.isPrimary` — so at most one datum is flagged at any
// time. Within a single datum only one of those fields can be set
// (discriminated union), so the check order inside `flaggedPrimary`
// is moot. If a future caller bypasses `setAxisRole` and creates
// multiple flagged datums, the first one in array order wins —
// deterministic but not semantic.
for (const d of datums) { for (const d of datums) {
if (d.type === "rectangle" && d.isAxisReference) { const flagged = flaggedPrimary(d)
return { kind: "rect", datum: d } if (flagged) return flagged
}
if (d.type === "line" && d.axisRole) {
return { kind: "line", datum: d }
}
if (d.type === "ellipse" && d.isPrimary) {
return { kind: "ellipse", datum: d }
}
} }
const typeRank = (d: Datum): number => const typeRank = (d: Datum): number =>

View File

@ -113,7 +113,13 @@ export const useAppStore = defineStore("app", () => {
id: string, id: string,
role: "rect" | "x" | "y" | "ellipse" | null, role: "rect" | "x" | "y" | "ellipse" | null,
) { ) {
// Clear any existing flag on other datums. // Only clear *other* datums' flags when actually assigning a new
// primary. A pure clear (`role === null`) must be a no-op against
// the rest of the set, otherwise clicking "None" on an unflagged
// line would silently strip the primary off whatever rect/ellipse
// legitimately holds it. The docstring guarantees "no-op if it
// wasn't set" for the target too — see the null branch below.
if (role !== null) {
for (let i = 0; i < datums.value.length; i++) { for (let i = 0; i < datums.value.length; i++) {
const d = datums.value[i] const d = datums.value[i]
if (!d || d.id === id) continue if (!d || d.id === id) continue
@ -125,6 +131,7 @@ export const useAppStore = defineStore("app", () => {
datums.value[i] = { ...d, isPrimary: false } datums.value[i] = { ...d, isPrimary: false }
} }
} }
}
const idx = datums.value.findIndex((d) => d.id === id) const idx = datums.value.findIndex((d) => d.id === id)
if (idx === -1) return if (idx === -1) return
const target = datums.value[idx] const target = datums.value[idx]
@ -134,7 +141,7 @@ export const useAppStore = defineStore("app", () => {
datums.value[idx] = { ...target, isAxisReference: false } datums.value[idx] = { ...target, isAxisReference: false }
} else if (target.type === "line") { } else if (target.type === "line") {
datums.value[idx] = { ...target, axisRole: null } datums.value[idx] = { ...target, axisRole: null }
} else { } else if (target.type === "ellipse") {
datums.value[idx] = { ...target, isPrimary: false } datums.value[idx] = { ...target, isPrimary: false }
} }
return return