Keep FileUpload labels associated with caller-supplied ids #46
jibril
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/fileupload-id-review"
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?
Passing an explicit id through FileUpload’s extra attributes replaces the native input id while its field label still targets the generated id. Resolve the caller’s id once for both the field label and native input, and remove id from the remaining splat, matching the other input components. Empty or null ids retain the generated fallback.
The avatar action layout in TeamAI/Shikigami#27 needs this supported id association for its enclosing Avatar label. Please review the component contract and regression coverage before merge.
Validation: this source tree is identical to the previously tested tree; serial solution build with no warnings and all 848 library tests passed. Regression cases cover generated, explicit, case-insensitive, empty and null ids.
Summary
Summary
Coverage
Kagaku.UI - 96.8%
Kagaku.UI.Fluxor - 97.7%
🔮 fufu~ Jibril reviewed your code!
Oh? A tiny little PR, +21/−3, two files… but fufu~ the smallest patches hide the sharpest bugs, and this one fixes a real one: a splatted
idused to silently overwrite the native input's id while the Field label kept pointing at the ghost of the generated one. Now both resolve through oneId. Label and control, bound together forever~ how romantic ♡Verdict: ✅ Looks good to me~
I read the full component, the full diff, and held this against its siblings. The
Idproperty is a character-for-character match ofInputFieldBase.FieldId's contract (src/Kagaku.UI/Components/InputFieldBase.cs:24) — sameSplat.Valuecase-insensitive resolution, same empty/null fallback, andSplat.Without(Extra, "id")strips the attribute so the splat can never fight the explicit one. Precisely the established pattern. ♪And I didn't take the tests on faith — I proved them. Three mutation probes, three corpses:
@attributes="Extra"→The_label_follows_the_callers_id_or_a_generated_fallbackfails on the""andnullcases. The strip is guarded~Id => _fieldId) → fails on bothavatarcases, including the"ID"variant. Caller-id resolution guarded~?? _fieldId→ the""case catches it. The{ Length: > 0 }guard is guarded~Zero survivors. Every new branch in this PR has a test that dies screaming if it breaks. That's how I like it ♡
💡 Little ideas (non-blocking)~
Extrato the right ofid="@_fieldId", so they carry exactly the bug this PR fixes: a caller-suppliedidleaves their Field label pointing at a vanished generated id. This PR is correct and scoped; the siblings just lag behindInputFieldBase's contract. Please give them the same treatment in a follow-up before Shikigami's avatar layout (Shikigami#27) reaches for a Select the same way~Idnow duplicatesInputFieldBase.FieldId's expression one-to-one. Two call sites of a one-liner is bearable (FileUpload rightly doesn't derive fromInputBase<>), but aSplat.Id(Extra, fallback)beside its siblings would fold them together next time someone adds a field component.✅ What I liked~
Field ForIdand the nativeInputFile id— the association can never split again. That's the architectural fix, not a symptom patch~"ID"), empty, null — and the default-render test now asserts the label/input association as a permanent invariant, not just for the fix.Automated review by Jibril · 2026-10-22
CI: green for head SHA
58b9ca13· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)