From b598eef8be76d4f502174116734494c35f6c7094 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Thu, 23 Apr 2026 11:46:26 +0200 Subject: [PATCH 1/4] InputWidget: stringify object values and use schema.parse for arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the "stringify a non-string value (e.g. event-set object)" branch from PropertyRow into InputWidget so the widget owns its (de)serialization (mirroring SelectWidget's pattern). Also replace the array parse from a plain value.split(',') to schema.parse(value), which additionally trims whitespace around each entry — "foo, bar" now yields ["foo", "bar"] instead of ["foo", " bar"]. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../components/elements/PropertyRow.jsx | 7 +--- src/editor/components/widgets/InputWidget.js | 40 ++++++++++++++----- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/editor/components/elements/PropertyRow.jsx b/src/editor/components/elements/PropertyRow.jsx index 8a16770c4..dd19370db 100644 --- a/src/editor/components/elements/PropertyRow.jsx +++ b/src/editor/components/elements/PropertyRow.jsx @@ -63,16 +63,11 @@ export default class PropertyRow extends React.Component { type = 'boolean'; } - let value = + const value = type === 'selector' ? props.entity.getDOMAttribute(props.componentname)?.[props.name] : props.data; - if (type === 'string' && value && typeof value !== 'string') { - // Allow editing a custom type like event-set component schema - value = props.schema.stringify(value); - } - const widgetProps = { name: props.name, onChange: function (name, value) { diff --git a/src/editor/components/widgets/InputWidget.js b/src/editor/components/widgets/InputWidget.js index 6cec96a2b..bad10f308 100644 --- a/src/editor/components/widgets/InputWidget.js +++ b/src/editor/components/widgets/InputWidget.js @@ -6,30 +6,48 @@ export default class InputWidget extends React.Component { id: PropTypes.string, name: PropTypes.string.isRequired, onChange: PropTypes.func, - value: PropTypes.any, - schema: PropTypes.object + schema: PropTypes.object, + value: PropTypes.any }; constructor(props) { super(props); - this.state = { value: this.props.value || '' }; + this.state = { value: this.stringifyValue(props.value) }; } - onChange = (e) => { - var value = e.target.value; - // if this component property is an array, then turn the string into an array - if (this.props.schema.type === 'array') { - value = value.split(','); + stringifyValue = (value) => { + // For selector and selectorAll types, getDOMAttribute returns null for + // single-property schema and undefined for multi-property schema when the + // property is not set. + if (value === undefined || value === null) return ''; + if (typeof value === 'string') return value; + // Non-string value (array, custom object like event-set): stringify for display + if (this.props.schema) return this.props.schema.stringify(value); + return String(value); + }; + + parseInput = (value) => { + // The type array doesn't bailout-on-string in its stringify + // (arrayStringify), so we need to parse the input value before calling + // onChange. That could potentially happen for a custom property that + // implements its own parse/stringify functions. + if (this.props.schema) { + return this.props.schema.parse(value); } + return value; + }; + + onChange = (e) => { + const value = e.target.value; this.setState({ value: value }); if (this.props.onChange) { - this.props.onChange(this.props.name, value); + this.props.onChange(this.props.name, this.parseInput(value)); } }; componentDidUpdate(prevProps) { if (this.props.value !== prevProps.value) { - this.setState({ value: this.props.value }); + this.setState({ value: this.stringifyValue(this.props.value) }); } } @@ -39,7 +57,7 @@ export default class InputWidget extends React.Component { id={this.props.id} type="text" className="string" - value={this.state.value || ''} + value={this.state.value} onChange={this.onChange} spellCheck="false" /> From b82be5a07882fa88ab9a74afee415c0e7d42c3c5 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Sat, 25 Apr 2026 12:50:53 +0200 Subject: [PATCH 2/4] EntityUpdateCommand: pass null through as newValue to support resetting a property Previously stringify(null) was called, which would fail or return "" depending on the property type. Pass null straight to updateEntity so it can route to removeAttribute and reset the property to its default. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/editor/lib/commands/EntityUpdateCommand.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/editor/lib/commands/EntityUpdateCommand.js b/src/editor/lib/commands/EntityUpdateCommand.js index 495305553..9d69b4371 100644 --- a/src/editor/lib/commands/EntityUpdateCommand.js +++ b/src/editor/lib/commands/EntityUpdateCommand.js @@ -34,9 +34,10 @@ export class EntityUpdateCommand extends Command { if (component) { if (payload.property) { if (component.schema[payload.property]) { - this.newValue = component.schema[payload.property].stringify( - payload.value - ); + this.newValue = + payload.value === null + ? null + : component.schema[payload.property].stringify(payload.value); this.oldValue = component.schema[payload.property].stringify( payload.entity.getAttribute(payload.component)[payload.property] ); @@ -57,9 +58,12 @@ export class EntityUpdateCommand extends Command { ); } } else { - this.newValue = component.isSingleProperty - ? component.schema.stringify(payload.value) - : payload.value; + this.newValue = + payload.value === null + ? null + : component.isSingleProperty + ? component.schema.stringify(payload.value) + : payload.value; this.oldValue = component.isSingleProperty ? component.schema.stringify( payload.entity.getAttribute(payload.component) From 95a4e595bf65394a8da441f2de7578531ce7f218 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Sat, 25 Apr 2026 12:53:02 +0200 Subject: [PATCH 3/4] PropertyRow: support selector and selectorAll via InputWidget on blur MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For selector and selectorAll types, read the value from getDOMAttribute (the raw selector string) rather than props.data (the parsed Element/NodeList), and commit on blur instead of on every keystroke — querying the DOM on each character is wasteful and a partial selector is rarely valid. Omit the schema when rendering InputWidget so the typed string is passed through as-is, and in EntityUpdateCommand skip the schema.stringify pass for selector types so A-Frame preserves the selector verbatim in attrValue, even if it doesn't resolve. Add onBlur and Enter-to-blur support to InputWidget so this flow works. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../components/elements/PropertyRow.jsx | 100 +++++++++++------- src/editor/components/widgets/InputWidget.js | 22 ++++ .../lib/commands/EntityUpdateCommand.js | 27 +++-- 3 files changed, 98 insertions(+), 51 deletions(-) diff --git a/src/editor/components/elements/PropertyRow.jsx b/src/editor/components/elements/PropertyRow.jsx index dd19370db..13ebec4de 100644 --- a/src/editor/components/elements/PropertyRow.jsx +++ b/src/editor/components/elements/PropertyRow.jsx @@ -63,49 +63,57 @@ export default class PropertyRow extends React.Component { type = 'boolean'; } - const value = - type === 'selector' - ? props.entity.getDOMAttribute(props.componentname)?.[props.name] - : props.data; + const isSelectorType = type === 'selector' || type === 'selectorAll'; - const widgetProps = { - name: props.name, - onChange: function (name, value) { - // Auto-switch to custom variant for building segments when modifying certain properties - const shouldSwitchToCustom = - // Surface changes on street-segment - (props.componentname === 'street-segment' && - props.name === 'surface') || - // Any changes to clone components (building-related) - props.componentname.startsWith('street-generated-clones'); - - if (shouldSwitchToCustom) { - const streetSegment = props.entity.getAttribute('street-segment'); - if ( - streetSegment && - streetSegment.type === 'building' && - streetSegment.variant !== 'custom' - ) { - // First switch to custom variant to prevent overrides - AFRAME.INSPECTOR.execute('entityupdate', { - entity: props.entity, - component: 'street-segment', - property: 'variant', - value: 'custom', - noSelectEntity: true - }); - } + const value = isSelectorType + ? props.entity.getDOMAttribute(props.componentname)?.[props.name] + : props.data; + + const updateProperty = (name, value) => { + // Auto-switch to custom variant for building segments when modifying certain properties + const shouldSwitchToCustom = + // Surface changes on street-segment + (props.componentname === 'street-segment' && + props.name === 'surface') || + // Any changes to clone components (building-related) + props.componentname.startsWith('street-generated-clones'); + + if (shouldSwitchToCustom) { + const streetSegment = props.entity.getAttribute('street-segment'); + if ( + streetSegment && + streetSegment.type === 'building' && + streetSegment.variant !== 'custom' + ) { + // First switch to custom variant to prevent overrides + AFRAME.INSPECTOR.execute('entityupdate', { + entity: props.entity, + component: 'street-segment', + property: 'variant', + value: 'custom', + noSelectEntity: true + }); } + } - AFRAME.INSPECTOR.execute('entityupdate', { - entity: props.entity, - component: props.componentname, - property: !props.isSingle ? props.name : '', - value: value, - noSelectEntity: props.noSelectEntity, - onEntityUpdate: props.onEntityUpdate - }); - }, + AFRAME.INSPECTOR.execute('entityupdate', { + entity: props.entity, + component: props.componentname, + property: !props.isSingle ? props.name : '', + value: value, + noSelectEntity: props.noSelectEntity, + onEntityUpdate: props.onEntityUpdate + }); + }; + + // For selector and selectorAll types, commit on blur only (not on each + // keystroke): a partial selector is rarely valid and querying the DOM on + // every character is wasteful. + const widgetProps = { + name: props.name, + ...(isSelectorType + ? { onBlur: updateProperty } + : { onChange: updateProperty }), value: value, id: this.id }; @@ -152,7 +160,17 @@ export default class PropertyRow extends React.Component { return ; } default: { - return ; + // For selector and selectorAll types, omit the schema so InputWidget + // doesn't parse the string into a DOM element / NodeList. We want the + // raw selector string to reach setAttribute — A-Frame preserves it + // verbatim in attrValue, even when it doesn't resolve, so the UI + // shows what the user typed. + return ( + + ); } } } diff --git a/src/editor/components/widgets/InputWidget.js b/src/editor/components/widgets/InputWidget.js index bad10f308..4144c76ac 100644 --- a/src/editor/components/widgets/InputWidget.js +++ b/src/editor/components/widgets/InputWidget.js @@ -5,6 +5,7 @@ export default class InputWidget extends React.Component { static propTypes = { id: PropTypes.string, name: PropTypes.string.isRequired, + onBlur: PropTypes.func, onChange: PropTypes.func, schema: PropTypes.object, value: PropTypes.any @@ -13,6 +14,7 @@ export default class InputWidget extends React.Component { constructor(props) { super(props); this.state = { value: this.stringifyValue(props.value) }; + this.input = React.createRef(); } stringifyValue = (value) => { @@ -45,6 +47,23 @@ export default class InputWidget extends React.Component { } }; + onBlur = (e) => { + if (this.props.onBlur) { + const value = e.target.value; + this.props.onBlur(this.props.name, this.parseInput(value)); + } + }; + + onKeyDown = (e) => { + e.stopPropagation(); + + // enter + if (e.keyCode === 13) { + this.input.current.blur(); + return; + } + }; + componentDidUpdate(prevProps) { if (this.props.value !== prevProps.value) { this.setState({ value: this.stringifyValue(this.props.value) }); @@ -55,10 +74,13 @@ export default class InputWidget extends React.Component { return ( ); diff --git a/src/editor/lib/commands/EntityUpdateCommand.js b/src/editor/lib/commands/EntityUpdateCommand.js index 9d69b4371..fb810712c 100644 --- a/src/editor/lib/commands/EntityUpdateCommand.js +++ b/src/editor/lib/commands/EntityUpdateCommand.js @@ -34,17 +34,26 @@ export class EntityUpdateCommand extends Command { if (component) { if (payload.property) { if (component.schema[payload.property]) { + const schemaProperty = component.schema[payload.property]; + const isSelectorType = + schemaProperty.type === 'selector' || + schemaProperty.type === 'selectorAll'; this.newValue = payload.value === null ? null - : component.schema[payload.property].stringify(payload.value); - this.oldValue = component.schema[payload.property].stringify( - payload.entity.getAttribute(payload.component)[payload.property] - ); + : isSelectorType + ? payload.value + : schemaProperty.stringify(payload.value); + this.oldValue = isSelectorType + ? (entity.getDOMAttribute(payload.component)?.[payload.property] ?? + '') + : schemaProperty.stringify( + entity.getAttribute(payload.component)[payload.property] + ); } else { // Just in case dynamic schema is not properly updated and we set an unknown property. I don't think this should happen. this.newValue = payload.value; - this.oldValue = payload.entity.getAttribute(payload.component)[ + this.oldValue = entity.getAttribute(payload.component)[ payload.property ]; } @@ -65,10 +74,8 @@ export class EntityUpdateCommand extends Command { ? component.schema.stringify(payload.value) : payload.value; this.oldValue = component.isSingleProperty - ? component.schema.stringify( - payload.entity.getAttribute(payload.component) - ) - : structuredClone(payload.entity.getDOMAttribute(payload.component)); + ? component.schema.stringify(entity.getAttribute(payload.component)) + : structuredClone(entity.getDOMAttribute(payload.component)); if (this.editor.config.debugUndoRedo) { console.log( 'entityupdate component', @@ -81,7 +88,7 @@ export class EntityUpdateCommand extends Command { } else { // id, class, mixin, data attributes this.newValue = payload.value; - this.oldValue = payload.entity.getAttribute(this.component); + this.oldValue = entity.getAttribute(this.component); if (this.editor.config.debugUndoRedo) { console.log( 'entityupdate attribute', From 6679f55c1eb550760a6bbcf2eee5c312e7dcce65 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Sat, 25 Apr 2026 13:00:27 +0200 Subject: [PATCH 4/4] in stringifyComponentValue check that property value is not undefined, this can happen if we reset a property and copy to clipboard --- src/editor/lib/entity.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/lib/entity.js b/src/editor/lib/entity.js index 050a26791..a4c489d1b 100644 --- a/src/editor/lib/entity.js +++ b/src/editor/lib/entity.js @@ -321,7 +321,7 @@ function stringifyComponentValue(schema, data) { function _multi() { var propertyBag = {}; Object.keys(data).forEach(function (name) { - if (schema[name]) { + if (schema[name] && data[name] !== undefined) { propertyBag[name] = schema[name].stringify(data[name]); } });