diff --git a/src/editor/components/elements/PropertyRow.jsx b/src/editor/components/elements/PropertyRow.jsx index 8a16770c4..13ebec4de 100644 --- a/src/editor/components/elements/PropertyRow.jsx +++ b/src/editor/components/elements/PropertyRow.jsx @@ -63,54 +63,57 @@ export default class PropertyRow extends React.Component { type = 'boolean'; } - let value = - type === 'selector' - ? props.entity.getDOMAttribute(props.componentname)?.[props.name] - : props.data; + const isSelectorType = type === 'selector' || type === 'selectorAll'; + + 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 + }); + } + } - if (type === 'string' && value && typeof value !== 'string') { - // Allow editing a custom type like event-set component schema - value = props.schema.stringify(value); - } + 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, - 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 - }); - } - } - - AFRAME.INSPECTOR.execute('entityupdate', { - entity: props.entity, - component: props.componentname, - property: !props.isSingle ? props.name : '', - value: value, - noSelectEntity: props.noSelectEntity, - onEntityUpdate: props.onEntityUpdate - }); - }, + ...(isSelectorType + ? { onBlur: updateProperty } + : { onChange: updateProperty }), value: value, id: this.id }; @@ -157,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 6cec96a2b..4144c76ac 100644 --- a/src/editor/components/widgets/InputWidget.js +++ b/src/editor/components/widgets/InputWidget.js @@ -5,31 +5,68 @@ export default class InputWidget extends React.Component { static propTypes = { id: PropTypes.string, name: PropTypes.string.isRequired, + onBlur: PropTypes.func, 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) }; + this.input = React.createRef(); } - 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)); + } + }; + + 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.props.value }); + this.setState({ value: this.stringifyValue(this.props.value) }); } } @@ -37,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 495305553..fb810712c 100644 --- a/src/editor/lib/commands/EntityUpdateCommand.js +++ b/src/editor/lib/commands/EntityUpdateCommand.js @@ -34,16 +34,26 @@ 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.oldValue = component.schema[payload.property].stringify( - payload.entity.getAttribute(payload.component)[payload.property] - ); + const schemaProperty = component.schema[payload.property]; + const isSelectorType = + schemaProperty.type === 'selector' || + schemaProperty.type === 'selectorAll'; + this.newValue = + payload.value === null + ? null + : 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 ]; } @@ -57,14 +67,15 @@ 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) - ) - : 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', @@ -77,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', 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]); } });