Repository navigation
fix(editor,kite3d): Resources headers match the panel, and the CLI keeps its execute bit - #44
Closed
pythonlearner1025 wants to merge 1 commit into
Closed
pythonlearner1025 wants to merge 1 commit into
pythonlearner1025 wants to merge 1 commit into
Conversation
…eps its execute bit The Resources tab's three section headers drew on var(--pt-app-background-color). In the dark theme that colour is lighter than the panel, so a collapsed header read as a selected row. The headers now draw on var(--main-app-background), the same variable the left panel draws on, so a collapsed header looks like any other row. Their hover rule named --pt-minimal-button-background-color-hover, which no stylesheet defines, so hover painted nothing. Hover now uses var(--pt-app-background-color): opaque, one step lighter, the lift a tree row shows. tsc writes dist/cli.js without the execute bit. A project that links the package by path keeps its bin symlink across rebuilds, so the next npx kite3d dev failed with "Permission denied" (exit 126) until someone ran chmod +x. The build script now chmods dist/cli.js to 755 after tsc. Proof: headless Chrome 153 on the terminator-next project, port 4472, CI=1, --no-open. Header and panel both read rgb(28, 33, 38) after the fix; the header read rgb(47, 52, 60) before it. Six theme combinations match. The sticky header stays opaque over scrolled rows. npm run typecheck, npm run lint, npm test -w packages/kite3d (5 files, 8 tests) and npm run test:scripts (2 tests) all pass. The new test fails on a build made with the old script.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What part this touches
Two small things, one in the editor and one in the CLI package.
The editor's left panel holds two tabs, Objects and Resources. The Resources tab draws three
sections: Materials, Textures and Geometries. Each section has a header row you click to open or
close it. The header sticks to the top of the panel while you scroll the list under it. Its styles
live in
packages/editor/src/renderer.scssat.resources-section-header. The markup lives inpackages/editor/src/components/BPResourcesTreeComponent.tsx.The second thing is the
kite3dpackage build.packages/kite3d/package.jsonbuildsdist/cli.jswith
tsc. That file is thekite3dcommand. A project that lists the package as a pathdependency gets a symlink at
node_modules/.bin/kite3dpointing straight at it.The bug
Two separate faults.
One, the headers read as selected. In the dark theme the three headers drew a light grey band
across the panel. Nothing was selected. The owner's words: "the colors are inverted - selected
should show light gray, not de-selected."
The rule was
background: var(--pt-app-background-color). That variable isrgb(47, 52, 60)inthe dark theme. The panel behind it is
rgb(28, 33, 38), which the editor defines itself as--main-app-backgroundand paints on.editorSplitContainer. So the header was two shades lighterthan the panel it sat on.
The hover rule was dead too. It named
--pt-minimal-button-background-color-hover. No stylesheetdefines that name. Blueprint's variable is
--minimal-button-background-color-hover, with nopt.An undefined custom property makes the whole
backgroundshorthand invalid, so hover fell back totransparent. I measured it in the browser: hover computed to
rgba(0, 0, 0, 0). The screenshotbefore-left-panel-hover.pngshows the result. The hovered header went dark and the two headersnext to it stayed light grey.
Two, the CLI loses its execute bit.
tscwritesdist/cli.jswith mode 644.npm installdoes set the bit, once, on the file the bin link resolves to. The failure comes after that. A
rebuild of this repo deletes
distand writes a fresh 644 file. The bin link in the linked projectstill points at it. The next command fails:
That is the loop the terminator project hits: install once, rebuild the editor, run
npx kite3d dev, get permission denied, runchmod +x, repeat.The fix
.resources-section-headerdraws onvar(--main-app-background), the variable the left panelalready draws on, so the two can never drift apart.
.resources-section-header:hoverdraws onvar(--pt-app-background-color), which is opaque andone step lighter than the panel. A translucent hover would let scrolled rows show through the
sticky header.
packages/kite3d/package.jsonbuild script appendsnode -e "require('fs').chmodSync('dist/cli.js', 0o755)"aftertsc.packages/kite3d/test/cli-bin.test.ts, assertsdist/cli.jshas the execute bit.Nothing else changed. No TypeScript, no new file in
scripts/, no lockfile change.The risk trade
The header no longer stands out from the rows. That is the point of the change, but it costs some
separation on a long list. The caret, the uppercase bold title and the count badge still mark a
header, and the hover lift marks it as clickable. The screenshots show it reads fine.
I changed the hover colour, which the brief asked me to keep. I kept the hover, not the broken
variable. The old rule painted nothing. Left alone, the header would have had no hover feedback at
all once the base matched the panel. The colour I chose blends to within 3 per channel of what a
Blueprint tree row shows on hover, and it is opaque, so the sticky header keeps hiding the rows
under it.
The rejected alternative for the hover was the real Blueprint variable,
--minimal-button-background-color-hover. It ishsla(216, 13%, 61%, .15), 15 percent opaque.On a sticky header that would let scrolled rows bleed through while the pointer is on it.
For the CLI, the rejected alternative was a script file under
scripts/. A one-linenode -einthe build script needs no new file and runs on every platform.
fs.chmodSyncdoes not throw onWindows, it just cannot set an execute bit there, which matches how Windows runs the
.cmdshim instead.
Tests
All runs are headless Google Chrome 153.0.8010.48, viewport 1512 by 982, device pixel ratio 2, a
fresh profile each run. Nothing took focus. The server ran with
CI=1and--no-open. I never rankite3d install.I confirmed both times that the served editor was my build. I fetched each bundle from the server
and matched its sha256 against
packages/editor/dist/assets. All three matched in both runs:served-is-my-build-before.txt(index-BrtAewVw.css) andserved-is-my-build-after.txt(
index-nlFnOqG8.css). The measure script also reads the served rule back. Before it returnedbackground: var(--pt-app-background-color), after it returnedbackground: var(--main-app-background).The colours I read, dark theme, all three sections collapsed
rgb(47, 52, 60)rgb(28, 33, 38)rgb(28, 33, 38)rgb(28, 33, 38)rgba(0, 0, 0, 0)rgb(47, 52, 60)rgb(45, 114, 210)rgb(45, 114, 210)All three headers read the same value in both runs.
One correction to the report. A selected resource row is Blueprint blue,
rgb(45, 114, 210), notlight grey. The grey the owner saw was the header's own background. A hovered row blends to about
rgb(45, 51, 57), which is close to the old header colour, so the headers looked permanentlyhovered or selected. Selection is unchanged by this PR.
Screenshots I looked at
/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/before-left-panel-collapsed.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/after-left-panel-collapsed.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/before-left-panel-hover.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/after-left-panel-hover.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/after-left-panel-selected.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/shots/after-left-panel-scrolled.png/private/tmp/claude-501/-Users-minjunes-blitz/278f5f97-7e16-4722-b1bc-c061014966db/scratchpad/evidence/resources-header/edge-shots/after-light-bpx-default.pngBefore: the three headers are one light grey band. After: they sit on the panel colour and only
the caret, the title and the count differ. Hovering one header lifts only that header. With
Materials open and the first material clicked, the row is blue and the header is not.
The sticky header
I scrolled the open Materials list to
scrollTop220 of 4044.document.elementFromPointon themiddle of the header returned the header, its opacity was 1, and its background was
rgb(28, 33, 38). No row shows through. Seeafter-left-panel-scrolled.png.The edges
edges.mjswalked six combinations: dark and light, times thebpx-default,bpx-flatandbpx-carbonBlueprint styler themes the settings menu offers. The header equalled the panel in allsix, and every hover colour was opaque and different from the panel.
Zero page errors in every run.
The execute bit
In a throwaway project whose only dependency is
"kite3d": "file:/Users/minjunes/kite3d-worktrees/resources-header/packages/kite3d":Failing, on a build made with the old script:
Passing, after
npm run build -w packages/kite3dwith the new script, with no chmod and noreinstall:
I also packed the package and read the tar entry, so the published tarball carries the bit:
Suites
npm run typecheck, exit 0.npm run lint, exit 0.npm test -w packages/kite3d, 5 files, 8 tests, all pass. It was 4 files and 7 tests before.npm run test:scripts, 2 tests, both pass.What fails without the fix: I ran the old build line verbatim,
rimraf dist && tsc -p tsconfig.build.json, thennpm test -w packages/kite3d. The new testfailed, 1 failed and 7 passed, exit 1, on
expect(mode & 0o111).toBe(0o111). I rebuilt with thenew script and it passed again.
There is no test for the colour.
packages/editorhas no test setup, so a CSS test would mean anew harness for one assertion. The headless run with the measured colours is the proof.
Deploy
The editor bundle ships in the next
kite3dnpm release, and the build script change ships withthe same release. The release runs
npm run buildthen publishes, sodist/cli.jsgets the bit onthe release machine.
Rollback: publish the previous version again, or
npm dist-tag add kite3d@0.21.0-alpha.4 latest.A user on a broken build can also run
chmod +x node_modules/kite3d/dist/cli.js, which is what thechange removes the need for.
The five questions
the linked-project CLI runs above.
asserts on it.
a variable that does not exist.
hover, light mode, and a rebuild after install for the CLI.
Anti-slop pass
I read my own diff against S1 to S10. I deleted the dead variable reference, which was S10, a patch
on a bad premise. I wrote no helper, no options object and no fallback. The comments say why, not
what.
rg "pt-minimal-button-background-color-hover"outsidenode_modulesanddistreturns nohits.