From 2299d034aed1c0993fae990fcf3ddaad3bae7c97 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 14 Jun 2026 19:08:33 -0500 Subject: fix(theme-studio): keep dropdown color names legible The color-picker popup colored each row's name and hex for contrast against the swatch, but the rows sit on the popup's fixed dark background. A mid or dark swatch (the blues past blue-1) got near-black text that vanished on the dark popup. The text now inherits the popup foreground for every real palette color. Only the solid "default" row, whose background is the color itself, still contrasts against its own fill. I moved the decision into dropdownRowTextColor with unit coverage, including a dark-swatch regression case. --- scripts/theme-studio/app-core.js | 13 ++++++++++++- scripts/theme-studio/app.js | 2 +- scripts/theme-studio/test-app-core.mjs | 20 +++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) (limited to 'scripts/theme-studio') diff --git a/scripts/theme-studio/app-core.js b/scripts/theme-studio/app-core.js index 2761031b9..d99e5e364 100644 --- a/scripts/theme-studio/app-core.js +++ b/scripts/theme-studio/app-core.js @@ -78,6 +78,17 @@ function resolveUiAttr(face,attr,uimap){ return null; } +// Text color for a swatch-dropdown popup row. A row showing a real palette color +// sits on the popup's own fixed background, so its name/hex text must inherit the +// popup foreground (return '' to use the CSS color). Coloring it for contrast +// against the swatch instead picks near-black text for a mid/dark swatch, which +// is unreadable on the dark popup. Only the "default" row, filled solid with +// SHOWN, uses a contrast color computed against that fill. +function dropdownRowTextColor(hex,shown,textOnFn){ + if(hex)return ''; + return shown?textOnFn(shown):''; +} + // Standard swatch-dropdown option list: a default entry, then the palette. When // cur is set but no longer in the palette, surface it as a "(gone)" entry first. function optList(cur,palette){const have=cur===''||palette.some(p=>p[0]===cur);return [['','— default —'],...(have?palette:[[cur,'(gone)'],...palette])];} @@ -367,4 +378,4 @@ function spanNeighborHex(cur,palette,ground,dir){ return null; } -export { nameToHex, normalizePkgFace, buildPkgmap, packagesForExport, mergePackagesInto, effResolve, resolveSyntaxFg, resolveUiAttr, optList, paletteOptionList, spanNeighborHex, slugify, ramp, fgSetFor, floor, lMax, COVERED_FACES, columnsFromPalette, regenColumn, rankByLightness, stepRepointPlan, sortColumns, sortColumnMembers, groundRoleOfEntry, groundColumnMembersFromPalette, clearPalettePlan, deletePaletteColumnPlan, areAllLocked, lockToggleLabel, toggleLockSet }; +export { nameToHex, normalizePkgFace, buildPkgmap, packagesForExport, mergePackagesInto, effResolve, resolveSyntaxFg, resolveUiAttr, dropdownRowTextColor, optList, paletteOptionList, spanNeighborHex, slugify, ramp, fgSetFor, floor, lMax, COVERED_FACES, columnsFromPalette, regenColumn, rankByLightness, stepRepointPlan, sortColumns, sortColumnMembers, groundRoleOfEntry, groundColumnMembersFromPalette, clearPalettePlan, deletePaletteColumnPlan, areAllLocked, lockToggleLabel, toggleLockSet }; diff --git a/scripts/theme-studio/app.js b/scripts/theme-studio/app.js index 4b331c555..5056a7be8 100644 --- a/scripts/theme-studio/app.js +++ b/scripts/theme-studio/app.js @@ -83,7 +83,7 @@ function mkColorDropdown(options,cur,onPick,opts={}){ const pop=document.createElement('div');pop.className='cddpop'; for(const [hex,name] of options){const row=document.createElement('div');row.className='cddrow'+(hex===cur?' sel':''); const shown=displayHex(hex),nm=hex?name:(opts.defaultName||name); - row.style.background=hex?'':shown;row.style.color=shown?textOn(shown):''; + row.style.background=hex?'':shown;row.style.color=dropdownRowTextColor(hex,shown,textOn); row.innerHTML=`${esc(nm)}${hex||shown||''}`; row.onclick=(ev)=>{ev.stopPropagation();cur=hex;paint();closeColorDropdown();onPick(hex);}; pop.appendChild(row);} diff --git a/scripts/theme-studio/test-app-core.mjs b/scripts/theme-studio/test-app-core.mjs index 63e79a95c..e98e511e5 100644 --- a/scripts/theme-studio/test-app-core.mjs +++ b/scripts/theme-studio/test-app-core.mjs @@ -7,7 +7,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { - nameToHex, normalizePkgFace, buildPkgmap, packagesForExport, mergePackagesInto, effResolve, resolveSyntaxFg, resolveUiAttr, optList, paletteOptionList, spanNeighborHex, slugify, + nameToHex, normalizePkgFace, buildPkgmap, packagesForExport, mergePackagesInto, effResolve, resolveSyntaxFg, resolveUiAttr, dropdownRowTextColor, optList, paletteOptionList, spanNeighborHex, slugify, clearPalettePlan, deletePaletteColumnPlan, groundColumnMembersFromPalette, areAllLocked, lockToggleLabel, toggleLockSet, } from './app-core.js'; import { planPaletteGenerator, entriesForGeneratedColumn } from './palette-generator-core.js'; @@ -769,3 +769,21 @@ test('resolveUiAttr: returns null when nothing up the chain is set', () => { test('resolveUiAttr: a face with no inherit and an unset attribute returns null', () => { assert.equal(resolveUiAttr('region', 'bg', { 'region': { bg: null } }), null); }); + +// dropdownRowTextColor: a popup row showing a real palette color inherits the +// popup foreground (legible on the fixed dark popup); only the filled default +// row uses a contrast color against its own background. textOn is stubbed so the +// test asserts the decision, not the contrast math. +const stubTextOn = (h) => (h === '#000000' ? '#fff' : '#000'); +test('dropdownRowTextColor: a real palette color inherits the popup fg (empty)', () => { + assert.equal(dropdownRowTextColor('#2a3a5a', '#2a3a5a', stubTextOn), ''); +}); +test('dropdownRowTextColor: a dark swatch still inherits (regression: blues were unreadable)', () => { + assert.equal(dropdownRowTextColor('#000000', '#000000', stubTextOn), ''); +}); +test('dropdownRowTextColor: the filled default row contrasts against its fill', () => { + assert.equal(dropdownRowTextColor('', '#cdced1', stubTextOn), '#000'); +}); +test('dropdownRowTextColor: a default row with no fill inherits (empty)', () => { + assert.equal(dropdownRowTextColor('', '', stubTextOn), ''); +}); -- cgit v1.2.3