feat(components): RadioCards and ColorField, a Slider that commits on release, Inter's italic face #30
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/editor-appearance-pieces"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
These are the library pieces for Tsumugi's editor appearance settings (Tsumugi ADR 0031, slice A1). Tsumugi adopts them with a submodule pointer bump in its next slice.
What's in it
RadioCards<TValue>+RadioCard<TValue>: one required choice out of a few. Each choice is a card whose content the caller draws throughOptionContent, for example a font shown in its own face, which a<select>cannot do.radiogroupofrole="radio"buttons, each named by itsLabel.radio.js(delegated, liketheme.js) moves the choice with the arrow keys, Home and End. It skips disabled cards and wraps around.--kg-radiocards-min, default12rem.ColorField: the native colour input on theFieldchrome.#rrggbb, ornullfor "the parent's default".Fallbackwith a dashed frame and the readout says "Default".aria-label"Reset: {Label}") hands backnull.Slider.CommitOnRelease: with it, the parent hears the value once per gesture (on change: the release, or a key press) instead of on every step of a drag.slider.jsmoves the readout through the drag, using labels the component renders for every step: theFormatted text asdata-kg-readouts, up to 1,000 steps, with the raw number past that.InterVariable-Italic.woff2from the Inter 4.1 release (the upright file we ship is byte-identical to that release's) gets its own@font-face, so<em>is drawn instead of slanted. The OFL licence ships beside the fonts asfonts/Inter-LICENSE.txt. Inter reserves no font name, and the files are unmodified.KagakuScriptsships the two new scripts; the existing registry test checks it againstwwwroot/js. The README mentions the new pieces.Tests
dotnet test: 534 + 15 passed. New tests pin:inputhandler (MissingEventHandlerException) and fires on change;aria-checkedand tab-stop sequences, with and without a choice;inputhandler;null;--kg-radiocards-minwith its12remdefault.The repo has no JS test harness, so I drove
slider.jsandradio.jsby hand in headless Chromium, on markup matching the components' output. Results:🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.UI - 95.9%
Kagaku.UI.Fluxor - 97.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Library pieces for Tsumugi's editor-appearance slice, and they're beautifully house-styled~ RadioCards wearing the same
Fieldchrome as Select, the roving-tab-stop radiogroup the ThemeSwitcher pioneered, a delegated script for every new behaviour… I read every line of the diff, every sibling it leans on, ran the full suite, and even fedslider.jsandradio.jsthrough my own little harness (15 scenarios, including the adversarial ones~). And I found one place where the maths and the markup disagree. Just one. But you know how I feel about those ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Kagaku.UI/wwwroot/js/slider.js:32-33— The readout can lie at the ends of an unaligned track.Math.round((value - min) / step)assumes every position maps to a loop index — but when the range isn't an exact number of steps ((Max − Min) % Step ≠ 0, e.g.min=0 max=10 step=3), the HTML range input clamps the last notch tomaxitself. At the rightmost notchvalue=10,Math.round(10/3) = 3,labels[3]= the readout of 9 — while the change event that fires on release commits 10. I drove this through my harness on the exact markup the component renders: readout says"9", parent receives10. A wrong readout during a drag is a UI showing a value it will not commit — exactly the class of lie this component exists to prevent. The same seam exists at the left edge (minnot on-step). [Theory] atSliderTests.cs:152pins the boundary at 999/1000 for the attribute, but the JS index maths has no pin at all.Fix: guard the label lookup —
var idx = Math.round((value - min) / step); var label = labels && idx >= 0 && idx < labels.length && Math.abs(min + idx * step - value) < step / 2 ? labels[idx] : null;…or simpler: have slider.js treat any value whose recomputedmin + idx*stepdiffers from the reported value beyond tolerance as off-array, falling back toinput.value(the raw number — always true). And pin it: a bUnit test asserting the rendereddata-kg-readoutsformin=0 max=10 step=3(labels stop at 9) plus the harness scenario showing the fallback engages at the max notch. Fufu~ you wouldn't leave a readout that fibs in production, would you? ♡💡 Little ideas (non-blocking)~
Slider.razor:81— TheMax < Minarm of theReadoutsbail returnsnull→ slider.js falls back to the raw number — correct behaviour. But no test reaches it: I removed the arm and the whole 534-test suite stayed green (verified mechanically). The left-edge case it guards against (thumb reportingminoff-step) is the same class as blocker #1; when you fix that, a[Theory]withMin > Maxand an off-stepMinwould pin both edges.Slider.razor:57-69— The two<input>branches differ only in the event hookup anddata-kg-*attributes. Once the guard from blocker #1 lives in slider.js, a single markup block with conditional attributes (data-kg-commit="@(CommitOnRelease ? "" : null)") could replace the branch — the CSS class list stays identical, so nothing user-visible changes. Only if you enjoy the symmetry as much as I do~<output>— spec-wiseformay reference multiple space-separated ids; you point it at the input only, which is right, and the label wiring test pins it. No action — just admiring the thoroughness. ♪💡 → none of these block.
✅ What I liked~
InterVariable-Italic.woff2=e564f652…and the upright =693b77d4…— both byte-identical to the release, andInter-LICENSE.txtis byte-identical to the upstreamLICENSE.txt. Unmodified, exactly as claimed. That's how you ship a font~Slider.CommitOnRelease— readout ownership staying with the render tree, JS only ever writingfirstChild.nodeValueso Blazor's diff keeps owning the text node — is genuinely elegant. My harness confirms the same node is mutated, never replaced (scenario 3), and the WeakMap cache keyed on the raw attribute handles the re-render case cleanly.radio.jsskip/wrap discipline: I fired every key at it — wrap-around skipping disabled cards both directions (d → bwhenais disabled), Home/End honouring the enabled set, non-radio keys ignored,preventDefaultonly on handled keys. 8/8 green. Therole="radio"promise is kept, matching theme.js's established pattern exactly.ColorField's refusal discipline: same-colour picks,rgb(...)garbage, and empty strings all refused (my probe removing the hex-validation arm went 4-red — the pins are real, not tautologies). Thearia-label="Reset: {Label}"naming and the disabled-hides-reset wiring are exactly the house style.>=, loop bound, unchanged-guard, hex-validation, re-click-guard) were killed on exactly their own pins.Automated review by Jibril · 2026-10-03
CI/CD: absent for head SHA
55bbcf4(PR opened 07:27, no coverage bot yet) · Local checks: 534 + 15 pass, 6 mutation probes (1 survivor disclosed), JS harness 15/15, fonts sha256-verified against upstream Inter 4.1Blockers: 1 · Non-blocking: 2 actionable
Addressed in
49dfcb2.⛔ The off-step readout.
slider.jsnow uses a step's label only when the value actually sits on that step:min + index·stepmust equal the value, withinstep/1000so decimal steps survive floating-point error, and the index must fall inside the labels. Otherwise the readout shows the rawinput.value, which is exactly what the change commits.The new bUnit pin asserts the readouts of a track that is not a whole number of steps:
0..10by3gives["0","3","6","9"].The JS itself I drove in headless Chromium:
1.2..2.4by0.05show their own label.💡 1, the
Max < Minarm: now pinned by its own test.💡 2, one input instead of two branches: I kept the branch. The branches differ in which event the input listens to,
oninputversusonchange. Splitting them is what keeps a commit-mode drag off the circuit: anoninputhandler on the element, even one that did nothing, would still send every step to the server.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? One commit, two files, thirty-four lines — and the lie I caught yesterday is dead. You even kept the branches and told me why keeping them is the whole point. Fufu~ you know how much I love a fix that arrives with its own reasoning attached ♡
Verdict: ✅ Looks good to me~
⛔ → ✅ The off-step readout, closed
slider.js:21-29— The newreadout()is exactly the right shape: a label only when the value sits on its step (min + index·stepwithinstep/1000— decimal-proof), the index inside the array, and the rawinput.valueotherwise — which is precisely what the change commits. I drove it through my harness on component-exact markup:"10", not 9's label. Round-1's lie, reproduced and dead. (My probe deleting theonStepguard goes red on exactly this —'9' !== '10'.)"9"shows,"9"commits — the two engine behaviors you and I measured, both honest now.Formatstill walks the drag — no regression.And the tolerance is load-bearing, not decoration: I zeroed it (
=== 0) and a formatted decimal track (0.1..0.3 by 0.1, labels"0.3 m") loses its label to the raw"0.3"— float error 5.55e-17 ≠ 0. Yourstep/1000is exactly wide enough. The 25-step 1.2..2.4 by 0.05 track keeps every label, matching your Chromium run.The pins are real.
A_track_that_is_not_a_whole_number_of_steps_has_labels_only_for_its_stepspins["0","3","6","9"]— my probe appending 10's label goes red on exactly it.A_track_whose_max_is_below_its_min_renders_no_readouts— my probe deleting theMax < Minarm goes red on exactly it. The round-1 survivor (the untested bail arm) is pinned at last ♪💡 Little ideas (non-blocking)~
SliderTests.cs:175/slider.js:19— both new comments say the track "ends on max itself" / "the browser's last notch is 10" as if universal; your own Chromium measurement (and mine) says Chromium ends on 9. The code is engine-agnostic — that's the point — but a future reader could take the comment as Chromium behavior. A "some engines clamp the last notch to max" would be timeless. Pure docs polish; not worth a round on its own.And declining my branch-merge idea with "an
oninputhandler, even a no-op, still sends every step to the circuit" — correct, and the best kind of correct: the one that explains itself. Keeping the branches is right ♡✅ What I liked~
readout(input)reads min/step/value fresh per event and falls back to the one source that can't lie (input.value), while the WeakMap cache still guards only the parsed JSON keyed on its raw attribute. The seam between cached and fresh is drawn exactly where it should be.Automated review by Jibril · 2026-10-03
CI/CD: stale for head
49dfcb2(coverage bot 10458 covers55bbcf4— awaiting refresh) · Local checks: 536 + 15 pass, 4/4 mutation probes killed on own pins (JS guard, Max<Min arm, off-step-label pin, decimal tolerance), 21/21 harness scenarios incl. round-1 regressionBlockers: 0 · Non-blocking: 1 docs-polish footnote