fix: input group clones with any id format, and unset row Settings checkboxes - #523
Merged
Merged
Conversation
The renderer cached each component under baseId() of its rendered id, which only strips the f- prefix when the id holds an 8-hex or UUID run, while "Add +" looked the row up by its raw id. Readable ids such as row-1 (the renderer docs' own example) missed the cache and the click threw "Cannot read properties of undefined (reading 'children')". Cache components under their rendered id and look rows and their children up by that same id. Name lookups for userFormData keep resolving both readable and generated names. Refs #520
…ecked
A row loaded with a config that leaves fieldset or inputGroup unset
(config: {}, or a partial config) replaces the row's default config, so
the Settings checkboxes got checked: undefined. dom renders an undefined
attribute value as an empty string, so the box showed as checked and the
first click saved false.
Coerce both values to booleans. Saved formData is unchanged: the
missing keys are still not added on load.
Refs #521
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fixes are consistent with existing behavior and have comprehensive regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes renderer input-group cloning for arbitrary IDs and corrects unset row settings checkboxes.
Changes:
- Caches renderer components by rendered ID for reliable cloning.
- Coerces unset row checkbox values to
false. - Adds unit and Playwright regression coverage and removes unused test code.
| File | Description |
|---|---|
src/lib/js/renderer/index.js |
Uses rendered IDs consistently for caching and cloning. |
src/lib/js/renderer/renderer.test.js |
Tests cloning across multiple ID formats. |
src/lib/js/renderer/option-groups.test.js |
Removes obsolete limitation comments. |
src/lib/js/renderer/layout-attrs.test.js |
Removes an obsolete limitation comment. |
src/lib/js/components/rows/row.js |
Coerces unset checkbox settings to unchecked. |
src/lib/js/components/rows/row.test.js |
Covers unset and partial row configurations. |
tests/renderer-input-group-ids.spec.js |
Adds end-to-end readable-ID cloning coverage. |
tests/row-settings-checkboxes.spec.js |
Adds end-to-end row checkbox coverage. |
tests/events.spec.js |
Removes an unused test variable. |
tests/editor-initialization.spec.js |
Removes an unused helper. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
Author
|
🎉 This PR is included in version 5.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Fixes #520
Fixes #521
#520: renderer input group "Add +" with readable ids
The renderer cached components under
baseId()of their rendered id.baseId()only strips thef-prefix when the id contains an 8-hex or UUID run, but "Add +" looked the row up by its raw id. With readable ids likerow-1(the renderer docs' example) the lookup missed and the click threwCannot read properties of undefined (reading 'children'). Readable ids that share a hex segment (row-deadbeef,field-deadbeef) also collided on the same key.Components are now cached under their rendered id (
f-{id}), and rows and their children are looked up by that same id, so cloning works for any id format.componentByName(used byuserFormData) still resolves both readable names and the hex-based generated names.#521: row Settings checkboxes checked when unset
A row loaded with
config: {}, or a partial config, gave its Fieldset and Input group checkboxeschecked: undefined.domrenders that aschecked="", so the boxes showed as checked and the first click savedfalse. Both are now coerced withBoolean(). Load doesn't add the missing keys, so saved formData is unchanged.I checked the other boolean checkboxes too. Field, column and attribute panel checkboxes go through
EditPanelItem, which already useschecked: !!valueand only renders keys that are set. A probe loading a row, column and fields withconfig: {}found no other checkbox showing checked for an unset value.Also
style(tests): removes two unused variables Biome flagged in e2e specs.Tests
renderer.test.js(readable, hex-segment,f--prefixed and hex ids),row.test.js(unset, partial, first click, unchanged save)renderer-input-group-ids.spec.jsandrow-settings-checkboxes.spec.jsfollow each issue's repro stepsnpm test834/834,npm run lint,npm run test:types, full Playwright suite 193 passed