Skip to content

Commit 844028f

Browse files
committed
Generate unique Palette harmony color names - PR_26156_179-palette-harmony-unique-generated-names
1 parent b2964fb commit 844028f

5 files changed

Lines changed: 139 additions & 21 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# PR_26156_179 Palette Harmony Unique Generated Names Report
2+
3+
## Result
4+
PASS
5+
6+
## Summary
7+
- Fixed Color Harmony Schemes add behavior so generated harmony names are unique in the active project palette.
8+
- Harmony suggestions keep stable display labels, such as `Complementary 1`, while add-time persistence increments the saved name when needed.
9+
- Existing generated harmony names now advance to the next available scheme number instead of failing duplicate-name validation.
10+
- Harmony additions skip existing RGB/hex colors before validation so repeated Add All calls do not create duplicate colors.
11+
- True user-entered duplicate names still use normal duplicate-name validation.
12+
13+
## Harmony Behavior
14+
- Two complementary generated suggestions that both start as `Complementary 1` save as `Complementary 1` and `Complementary 2`.
15+
- Add All adds all non-duplicate generated harmony colors.
16+
- Repeating Add All skips existing harmony hex values without surfacing duplicate-name validation.
17+
- Single pin/dot harmony add behavior uses the same generated-name allocator.
18+
19+
## Validation Boundary
20+
- Duplicate-name validation remains unchanged for manual user swatch add/update paths.
21+
- The duplicate-name bypass is not global; only auto-generated harmony names are incremented.
22+
- Duplicate RGB/hex protection remains active by pre-skipping existing harmony colors.
23+
24+
## Validation
25+
- `node --check toolbox/colors/colors.js`
26+
- PASS
27+
- `node --check toolbox/colors/palette-workspace-repository.js`
28+
- PASS
29+
- `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
30+
- PASS
31+
- `rg "style=|<style|onclick|onchange|oninput|onsubmit" toolbox/colors/index.html toolbox/colors/colors.js toolbox/colors/palette-workspace-repository.js tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
32+
- PASS, no matches
33+
- `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs --project=playwright --workers=1 --reporter=list`
34+
- PASS, 3 tests
35+
- `npm run test:playwright:static`
36+
- PASS
37+
- `git diff --check`
38+
- PASS
39+
40+
## Verification Coverage
41+
- Adding complementary harmony colors creates unique generated names.
42+
- Add All adds all non-duplicate harmony colors.
43+
- Repeated Add All does not duplicate existing hex values.
44+
- Duplicate-name validation still blocks true user-entered duplicate names.
45+
- Harmony duplicate-name validation does not appear for auto-generated names that can be safely incremented.
46+
47+
## Skipped
48+
- Full samples smoke was not run, per BUILD instruction.

docs_build/dev/reports/playwright_v8_coverage_report.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,19 @@ Note: entry percentages use function coverage when available, otherwise line cov
1212
Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.
1313

1414
Exercised tool entry points detected:
15-
(86%) Toolbox Index - exercised 5 runtime JS files
15+
(85%) Toolbox Index - exercised 5 runtime JS files
1616
(0%) Tool Template V2 - not exercised by this Playwright run
1717
(83%) Theme V2 Shared JS - exercised 2 runtime JS files
1818

1919
Changed runtime JS files covered:
20-
(95%) toolbox/colors/colors.js - executed lines 793/793; executed functions 77/81
20+
(87%) toolbox/colors/palette-workspace-repository.js - executed lines 1218/1218; executed functions 116/134
2121

2222
Files with executed line/function counts where available:
2323
(65%) toolbox/project-workspace/project-workspace-mock-repository.js - executed lines 402/402; executed functions 20/31
2424
(75%) toolbox/toolRegistry.js - executed lines 1754/1754; executed functions 27/36
2525
(80%) assets/theme-v2/js/gamefoundry-partials.js - executed lines 205/205; executed functions 16/20
2626
(87%) assets/theme-v2/js/tool-display-mode.js - executed lines 201/201; executed functions 13/15
27-
(87%) toolbox/colors/palette-workspace-repository.js - executed lines 1190/1190; executed functions 113/130
27+
(87%) toolbox/colors/palette-workspace-repository.js - executed lines 1218/1218; executed functions 116/134
2828
(95%) toolbox/colors/colors.js - executed lines 793/793; executed functions 77/81
2929
(100%) toolbox/colors/palette-source-mock-db.js - executed lines 927/927; executed functions 6/6
3030

@@ -33,4 +33,4 @@ Uncovered or low-coverage changed JS files:
3333

3434
Changed JS files considered:
3535
(0%) tests/playwright/tools/PaletteToolMockRepository.spec.mjs - changed JS file not collected as browser runtime coverage
36-
(95%) toolbox/colors/colors.js - changed JS file with browser V8 coverage
36+
(87%) toolbox/colors/palette-workspace-repository.js - changed JS file with browser V8 coverage

docs_build/dev/reports/testing_lane_execution_report.md

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
# PR_26156_178 Testing Lane Execution Report
1+
# PR_26156_179 Testing Lane Execution Report
22

33
## Result
44
PASS
55

66
## Commands Run
77
- `node --check toolbox/colors/colors.js`
88
- PASS
9+
- `node --check toolbox/colors/palette-workspace-repository.js`
10+
- PASS
911
- `node --check tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
1012
- PASS
11-
- `rg "placeholder=" toolbox/colors/index.html tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
12-
- PASS, no matches
1313
- `rg "style=|<style|onclick|onchange|oninput|onsubmit" toolbox/colors/index.html toolbox/colors/colors.js toolbox/colors/palette-workspace-repository.js tests/playwright/tools/PaletteToolMockRepository.spec.mjs`
1414
- PASS, no matches
1515
- `npx playwright test tests/playwright/tools/PaletteToolMockRepository.spec.mjs --project=playwright --workers=1 --reporter=list`
@@ -20,26 +20,25 @@ PASS
2020
- PASS
2121

2222
## Required Lanes
23-
- Targeted Palette Tool runtime/UI lane: PASS.
23+
- Targeted Palette Tool harmony runtime lane: PASS.
2424
- Changed-file/static validation: PASS.
2525

2626
## Playwright Coverage
27-
- Initial no-selection state shows empty disabled Swatch Editor fields: PASS.
28-
- Initial no-selection state shows no placeholder text in Swatch Editor fields: PASS.
29-
- Placeholder text is absent from Swatch Editor and User Defined Swatch inputs: PASS.
30-
- Selecting a source-backed swatch populates Swatch Editor selected values: PASS.
31-
- Selecting a user-defined swatch populates Swatch Editor selected values: PASS.
32-
- Removing the selected swatch returns Swatch Editor fields to empty disabled state: PASS.
33-
- PR_26156_177 User Defined Swatch rules remain covered: PASS.
27+
- Adding complementary harmony colors creates unique generated names: PASS.
28+
- Generated complementary suggestions that start as `Complementary 1` save as `Complementary 1` and `Complementary 2`: PASS.
29+
- Add All adds all non-duplicate generated harmony colors: PASS.
30+
- Repeated Add All skips existing harmony hex values without duplicates: PASS.
31+
- Duplicate-name validation still blocks true user-entered duplicate names: PASS.
32+
- Auto-generated harmony duplicate names are incremented instead of surfacing duplicate-name validation: PASS.
3433

3534
## Impacted Lane
36-
- Palette Tool runtime/UI lane.
35+
- Palette Tool harmony runtime lane.
3736
- Changed-file/static validation lane.
3837

3938
## Skipped Lanes
4039
- Full samples smoke was skipped by BUILD instruction.
41-
- Shared Tool Center fullscreen UI lane was skipped because PR178 does not modify shared Tool Center or Theme V2 behavior.
42-
- Broader tool lanes were skipped because changes are confined to Palette Tool input markup and targeted Palette spec assertions.
40+
- Shared Tool Center fullscreen UI lane was skipped because PR179 does not modify shared Tool Center or Theme V2 behavior.
41+
- Broader tool lanes were skipped because changes are confined to Palette harmony repository behavior and targeted Palette spec assertions.
4342

4443
## Not Run
4544
- Full samples smoke was not run, per BUILD instruction.

tests/playwright/tools/PaletteToolMockRepository.spec.mjs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,9 @@ test("Palette repository owns active project swatches without mutating invalid p
262262
]));
263263

264264
expect(repository.addSwatch({ symbol: "H", hex: "#654321", name: "Other" }).ok).toBe(false);
265-
expect(repository.addSwatch({ symbol: "O", hex: "#654321", name: "Hero Blue" }).ok).toBe(false);
265+
const duplicateNameResult = repository.addSwatch({ symbol: "O", hex: "#654321", name: "Hero Blue" });
266+
expect(duplicateNameResult.ok).toBe(false);
267+
expect(duplicateNameResult.issues.map((issue) => issue.label)).toContain("Duplicate Name");
266268
expect(repository.addSwatch({ symbol: "O", hex: "#123456", name: "Other" }).ok).toBe(false);
267269

268270
const updateResult = repository.updateSelectedSwatch({
@@ -331,8 +333,45 @@ test("Palette repository owns active project swatches without mutating invalid p
331333
expect(harmonySwatch).toMatchObject({ source: "harmony", tags: [] });
332334
const duplicateHarmonyResult = repository.addHarmonySuggestions([harmonySuggestion]);
333335
expect(duplicateHarmonyResult.ok).toBe(false);
336+
expect(duplicateHarmonyResult.issues).toEqual([]);
334337
expect(repository.listSwatches().filter((swatch) => swatch.name === harmonySuggestion.name)).toHaveLength(1);
335338

339+
const harmonyRepository = createProjectWorkspacePaletteRepository({ sourcePaletteRows });
340+
expect(harmonyRepository.addSwatch({ symbol: "A", hex: "#112233", name: "Base One" }).ok).toBe(true);
341+
expect(harmonyRepository.addSwatch({ symbol: "B", hex: "#445566", name: "Base Two" }).ok).toBe(true);
342+
const firstComplementary = harmonyRepository.createHarmonySuggestions(harmonyRepository.findSwatch("A"), {
343+
matchSource: "calculated",
344+
schemeId: "complementary"
345+
})[0];
346+
const secondComplementary = harmonyRepository.createHarmonySuggestions(harmonyRepository.findSwatch("B"), {
347+
matchSource: "calculated",
348+
schemeId: "complementary"
349+
})[0];
350+
expect(firstComplementary.name).toBe("Complementary 1");
351+
expect(secondComplementary.name).toBe("Complementary 1");
352+
expect(harmonyRepository.addHarmonySuggestion(firstComplementary).ok).toBe(true);
353+
expect(harmonyRepository.addHarmonySuggestion(secondComplementary).ok).toBe(true);
354+
expect(harmonyRepository.listSwatches().filter((swatch) => swatch.source === "harmony").map((swatch) => swatch.name)).toEqual([
355+
"Complementary 1",
356+
"Complementary 2"
357+
]);
358+
const triadicSuggestions = harmonyRepository.createHarmonySuggestions(harmonyRepository.findSwatch("A"), {
359+
matchSource: "calculated",
360+
schemeId: "triadic"
361+
});
362+
const triadicAddAllResult = harmonyRepository.addHarmonySuggestions(triadicSuggestions);
363+
expect(triadicAddAllResult.ok).toBe(true);
364+
expect(triadicAddAllResult.message).toBe("Harmony add complete: 2 added, 0 skipped.");
365+
expect(harmonyRepository.listSwatches().filter((swatch) => swatch.name.startsWith("Triadic ")).map((swatch) => swatch.name)).toEqual([
366+
"Triadic 1",
367+
"Triadic 2"
368+
]);
369+
const repeatTriadicResult = harmonyRepository.addHarmonySuggestions(triadicSuggestions);
370+
expect(repeatTriadicResult.ok).toBe(false);
371+
expect(repeatTriadicResult.message).toBe("Harmony add complete: 0 added, 2 skipped.");
372+
expect(repeatTriadicResult.issues).toEqual([]);
373+
expect(harmonyRepository.listSwatches().filter((swatch) => swatch.name.startsWith("Triadic "))).toHaveLength(2);
374+
336375
repository.selectSwatch("R");
337376
repository.recordSwatchUsage({ assetId: "asset.color.red", symbol: "R", toolId: "assets" });
338377
const blockedRemoval = repository.removeSelectedSwatch();

toolbox/colors/palette-workspace-repository.js

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,28 @@ function nextAvailableSymbol(existingSwatches, seedText = "") {
527527
return candidates.find((candidate) => !used.has(candidate)) || "";
528528
}
529529

530+
function generatedHarmonyNameRoot(name) {
531+
const normalizedName = normalizeText(name);
532+
return normalizedName.replace(/\s+\d+$/, "") || normalizedName;
533+
}
534+
535+
function nextGeneratedHarmonyName(name, existingSwatches) {
536+
const normalizedName = normalizeText(name);
537+
const usedNames = new Set(existingSwatches.map((swatch) => swatch.name.toLowerCase()));
538+
if (normalizedName && !usedNames.has(normalizedName.toLowerCase())) {
539+
return normalizedName;
540+
}
541+
542+
const root = generatedHarmonyNameRoot(normalizedName) || "Harmony";
543+
let index = 1;
544+
let candidate = `${root} ${index}`;
545+
while (usedNames.has(candidate.toLowerCase())) {
546+
index += 1;
547+
candidate = `${root} ${index}`;
548+
}
549+
return candidate;
550+
}
551+
530552
function createUsageId(projectId, swatchSymbol, assetId) {
531553
return `${projectId || "project"}-${swatchSymbol || "swatch"}-${assetId || "asset"}`;
532554
}
@@ -996,7 +1018,6 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
9961018
function addHarmonySuggestion(suggestion = {}) {
9971019
const projectId = activeProjectId();
9981020
const swatches = getActiveSwatches();
999-
const symbol = nextAvailableSymbol(swatches, suggestion.name);
10001021
if (!projectId) {
10011022
return {
10021023
issues: [createIssue("activeProject", "Active Project", "Open a project before adding harmony colors.")],
@@ -1005,6 +1026,17 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
10051026
snapshot: getSnapshot()
10061027
};
10071028
}
1029+
if (rgbKey(suggestion.hex) && swatches.some((swatch) => rgbKey(swatch.hex) === rgbKey(suggestion.hex))) {
1030+
return {
1031+
issues: [],
1032+
ok: false,
1033+
message: "Harmony color already exists in the active project palette.",
1034+
snapshot: getSnapshot()
1035+
};
1036+
}
1037+
1038+
const name = nextGeneratedHarmonyName(suggestion.name, swatches);
1039+
const symbol = nextAvailableSymbol(swatches, name);
10081040
if (!symbol) {
10091041
return {
10101042
issues: [createIssue("symbol", "Symbol", "No available one-character palette symbol remains.")],
@@ -1016,7 +1048,7 @@ export function createProjectWorkspacePaletteRepository(options = {}) {
10161048

10171049
const validation = validatePaletteSwatchInput({
10181050
hex: suggestion.hex,
1019-
name: suggestion.name,
1051+
name,
10201052
source: "harmony",
10211053
symbol,
10221054
tags: []

0 commit comments

Comments
 (0)