Skip to content

Commit 19c2984

Browse files
committed
feat(variables): template-aware bindings (counts + rename ripple)
VariablesPanel's per-row binding count now includes `«name»` template references in addition to single-bind `variableId` references, so the user sees real usage of each Variable across both binding styles. De-duped per object — a field that carries both `variableId === V` and `«V»` in its content counts as one place, not two. updateVariable rewrites every `«oldName»` marker in every object's content to `«newName»` when a Variable is renamed — without this ripple, renames would silently dangle the templates (resolve to literal text). Identity-preserving at the leaf level so React memoisation downstream stays effective for objects whose content didn't carry the renamed name. Reuses the shared `getObjectStringContent` helper from variableBinding.ts so the content-reading shape check stays in one place. 3 new tests cover the rename ripple (multiple markers, identity preservation, no-op when name unchanged).
1 parent d5e6422 commit 19c2984

3 files changed

Lines changed: 149 additions & 5 deletions

File tree

‎src/components/Variables/VariablesPanel.tsx‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import { ConfirmDialog } from '../ui/ConfirmDialog';
2222
import { FieldLabel } from '../ui/FieldLabel';
2323
import { useT } from '../../lib/useT';
2424
import type { Translations } from '../../locales';
25-
import { getVariableSource, type VariableSource } from '../../lib/variableBinding';
25+
import { getObjectStringContent, getVariableSource, type VariableSource } from '../../lib/variableBinding';
26+
import { extractTemplateRefs } from '../../lib/fnTemplate';
2627
import { VariableSourceBadge } from './VariableSourceBadge';
2728

2829
export function VariablesPanel() {
@@ -487,18 +488,36 @@ function formatDeleteMessage(
487488
}
488489

489490
/** Walk every page (groups too) and tally how many fields reference each
490-
* variable. Returns a Map keyed by variable.id. Variables with no
491-
* bindings are absent from the map; callers default to 0. */
491+
* variable, either via single-bind `variableId` OR via inline
492+
* `«name»` template markers in their content. Returns a Map keyed by
493+
* variable.id. Variables with no bindings are absent; callers
494+
* default to 0. */
492495
function countBindings(
493496
pages: { objects: LabelObject[] }[],
494497
variables: readonly Variable[],
495498
): Map<string, number> {
496499
const known = new Set(variables.map((v) => v.id));
500+
const byName = new Map(variables.map((v) => [v.name, v.id]));
497501
const counts = new Map<string, number>();
498502
for (const page of pages) {
499503
for (const obj of walkObjects(page.objects)) {
504+
// De-dupe per OBJECT across both binding styles: a field with
505+
// both `variableId === V` and `«V»` in its content counts as
506+
// one usage of V, not two. Mirrors how the user thinks about
507+
// "where is V used" — one field = one place.
508+
const refsInThisObj = new Set<string>();
500509
if (obj.variableId && known.has(obj.variableId)) {
501-
counts.set(obj.variableId, (counts.get(obj.variableId) ?? 0) + 1);
510+
refsInThisObj.add(obj.variableId);
511+
}
512+
const c = getObjectStringContent(obj);
513+
if (c !== undefined) {
514+
for (const name of extractTemplateRefs(c)) {
515+
const id = byName.get(name);
516+
if (id) refsInThisObj.add(id);
517+
}
518+
}
519+
for (const id of refsInThisObj) {
520+
counts.set(id, (counts.get(id) ?? 0) + 1);
502521
}
503522
}
504523
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { describe, it, expect, beforeEach } from "vitest";
2+
import { useLabelStore } from "./labelStore";
3+
import type { LabelObject } from "../types/Group";
4+
5+
const newTextObj = (id: string, content: string): LabelObject =>
6+
({
7+
id,
8+
type: "text",
9+
x: 0,
10+
y: 0,
11+
rotation: 0,
12+
props: {
13+
content,
14+
fontHeight: 20,
15+
fontWidth: 0,
16+
rotation: "N",
17+
},
18+
}) as unknown as LabelObject;
19+
20+
describe("labelStore — template marker rename ripple", () => {
21+
beforeEach(() => {
22+
useLabelStore.setState({
23+
variables: [],
24+
pages: [{ objects: [] }],
25+
currentPageIndex: 0,
26+
} as Partial<ReturnType<typeof useLabelStore.getState>>);
27+
});
28+
29+
it("rewrites every «oldName» marker when a variable is renamed", () => {
30+
const id = useLabelStore.getState().addVariable({ name: "sku" });
31+
expect(id).not.toBeNull();
32+
useLabelStore.setState({
33+
pages: [
34+
{
35+
objects: [
36+
newTextObj("a", "lone «sku»"),
37+
newTextObj("b", "two «sku» and «sku» again"),
38+
newTextObj("c", "untouched literal"),
39+
],
40+
},
41+
],
42+
} as Partial<ReturnType<typeof useLabelStore.getState>>);
43+
44+
useLabelStore.getState().updateVariable(id!, { name: "product" });
45+
46+
const objs = useLabelStore.getState().pages[0]!.objects as LabelObject[];
47+
expect((objs[0] as { props: { content: string } }).props.content)
48+
.toBe("lone «product»");
49+
expect((objs[1] as { props: { content: string } }).props.content)
50+
.toBe("two «product» and «product» again");
51+
expect((objs[2] as { props: { content: string } }).props.content)
52+
.toBe("untouched literal");
53+
});
54+
55+
it("preserves object identity when the rename does not touch a leaf", () => {
56+
const id = useLabelStore.getState().addVariable({ name: "sku" });
57+
const before = newTextObj("a", "no markers here");
58+
useLabelStore.setState({
59+
pages: [{ objects: [before] }],
60+
} as Partial<ReturnType<typeof useLabelStore.getState>>);
61+
62+
useLabelStore.getState().updateVariable(id!, { name: "product" });
63+
64+
const after = useLabelStore.getState().pages[0]!.objects[0];
65+
expect(after).toBe(before);
66+
});
67+
68+
it("leaves markers untouched when the rename target name is unchanged", () => {
69+
const id = useLabelStore.getState().addVariable({ name: "sku" });
70+
const obj = newTextObj("a", "value: «sku»");
71+
useLabelStore.setState({
72+
pages: [{ objects: [obj] }],
73+
} as Partial<ReturnType<typeof useLabelStore.getState>>);
74+
75+
useLabelStore.getState().updateVariable(id!, { name: "sku" });
76+
77+
const after = useLabelStore.getState().pages[0]!.objects[0];
78+
expect(after).toBe(obj);
79+
});
80+
});

‎src/store/labelStore.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {
1919
} from '../types/Group';
2020
import { locales } from '../locales';
2121
import type { LocaleCode } from '../locales';
22+
import { renameTemplateMarker } from '../lib/fnTemplate';
23+
import { getObjectStringContent } from '../lib/variableBinding';
2224
import { isDefaultLabelaryHost, fetchPreview, labelaryErrorMessage } from '../lib/labelary';
2325
import {
2426
nextFreeFnNumber,
@@ -57,6 +59,35 @@ function isLockBypass(changes: ObjectChanges): boolean {
5759
return keys.length > 0 && keys.every((k) => LOCK_BYPASS_KEYS.has(k));
5860
}
5961

62+
/** Apply `renameTemplateMarker` to every leaf's `content` in a subtree.
63+
* Identity-preserving: returns the same array (and same node refs)
64+
* when no markers needed rewriting, so React memoisation downstream
65+
* stays effective for the common case where the rename touched no
66+
* templates. */
67+
function rewriteTemplateMarkers(
68+
objects: LabelObject[],
69+
oldName: string,
70+
newName: string,
71+
): LabelObject[] {
72+
let changed = false;
73+
const next = objects.map((obj) => {
74+
if (isGroup(obj)) {
75+
const nextChildren = rewriteTemplateMarkers(obj.children, oldName, newName);
76+
if (nextChildren === obj.children) return obj;
77+
changed = true;
78+
return { ...obj, children: nextChildren };
79+
}
80+
const content = getObjectStringContent(obj);
81+
if (content === undefined) return obj;
82+
const renamed = renameTemplateMarker(content, oldName, newName);
83+
if (renamed === content) return obj;
84+
changed = true;
85+
const props = (obj as { props: object }).props;
86+
return { ...obj, props: { ...props, content: renamed } } as LabelObject;
87+
});
88+
return changed ? next : objects;
89+
}
90+
6091
function applyObjectChanges(
6192
obj: LabelObject,
6293
changes: ObjectChanges,
@@ -1109,9 +1140,23 @@ export const useLabelStore = create<LabelState>()(
11091140
if (state.variables.some((v) => v.id !== id && v.fnNumber === changes.fnNumber)) return {};
11101141
}
11111142

1112-
return {
1143+
const next: Partial<typeof state> = {
11131144
variables: state.variables.map((v) => (v.id === id ? { ...v, ...changes } : v)),
11141145
};
1146+
// Rename ripple: when the variable's name changes, every
1147+
// `«oldName»` marker in any object's content needs to point
1148+
// at the new name. Without this the templates dangle (resolve
1149+
// to literal text) and the user has no obvious way to fix
1150+
// them other than re-typing.
1151+
if (changes.name !== undefined && changes.name !== existing.name) {
1152+
const oldName = existing.name;
1153+
const newName = changes.name;
1154+
next.pages = state.pages.map((page) => ({
1155+
...page,
1156+
objects: rewriteTemplateMarkers(page.objects, oldName, newName),
1157+
}));
1158+
}
1159+
return next;
11151160
}),
11161161

11171162
setVariables: (variables) =>

0 commit comments

Comments
 (0)