diff --git a/src/components/components/CommonComponents.js b/src/components/components/CommonComponents.js index 9868791fc..0ac8a4319 100644 --- a/src/components/components/CommonComponents.js +++ b/src/components/components/CommonComponents.js @@ -12,20 +12,44 @@ import Events from '../../lib/Events'; import { saveBlob } from '../../lib/utils'; import GLTFIcon from '../../../assets/gltf.svg'; -// @todo Take this out and use updateEntity? -function changeId(componentName, value) { - var entity = AFRAME.INSPECTOR.selectedEntity; - if (entity.id !== value) { - entity.id = value; - Events.emit('entityidchange', entity); - } -} - export default class CommonComponents extends React.Component { static propTypes = { entity: PropTypes.object }; + state = { + duplicateId: null, + idInputKey: 0 + }; + + changeId = (componentName, value) => { + var entity = AFRAME.INSPECTOR.selectedEntity; + if (entity.id !== value) { + // Ids must be unique; committing a duplicate id breaks entity lookups + // done with getElementById or querySelector in components. Refuse the + // value, show a message and remount the input so it displays the + // entity's current id again. + if (value && document.getElementById(value)) { + this.setState((state) => ({ + duplicateId: value, + idInputKey: state.idInputKey + 1 + })); + return; + } + this.setState({ duplicateId: null }); + entity.id = value; + Events.emit('entityidchange', entity); + } else { + this.setState({ duplicateId: null }); + } + }; + + componentDidUpdate(prevProps) { + if (prevProps.entity !== this.props.entity && this.state.duplicateId) { + this.setState({ duplicateId: null }); + } + } + onEntityUpdate = (detail) => { if (detail.entity !== this.props.entity) { return; @@ -125,12 +149,19 @@ export default class CommonComponents extends React.Component { ID + {this.state.duplicateId !== null && ( +
+ The id "{this.state.duplicateId}" is already used by + another element, the previous id was restored. +
+ )}
{entity.getAttribute('class')} diff --git a/src/lib/raycaster.js b/src/lib/raycaster.js index b3ed4a3eb..58b947db2 100644 --- a/src/lib/raycaster.js +++ b/src/lib/raycaster.js @@ -73,7 +73,14 @@ export function initRaycaster(inspector) { if (event instanceof CustomEvent) { return; } - event.preventDefault(); + // Don't preventDefault on left click: it would suppress the browser's + // default focus transfer, leaving a focused panel input (e.g. the id + // field) focused while the selection changes underneath it, so its blur + // handler would later commit a stale value to the newly selected entity. + if (event.button !== 0) { + // Middle click: prevent autoscroll so it can be used for zoom. + event.preventDefault(); + } const array = getMousePosition( inspector.container, event.clientX, diff --git a/src/style/components.styl b/src/style/components.styl index 7dd16f42b..867c00422 100644 --- a/src/style/components.styl +++ b/src/style/components.styl @@ -137,6 +137,11 @@ .text width 90px +.propertyRowError + color var(--color-error) + font-size 13px + padding 2px 30px 2px 15px + .propertyRowDefined .text color var(--color-property-defined) font-weight 600 diff --git a/src/style/index.styl b/src/style/index.styl index 0981cfc55..44a432a46 100644 --- a/src/style/index.styl +++ b/src/style/index.styl @@ -12,6 +12,11 @@ body.aframe-inspector-opened margin 0 overflow hidden + a-scene .a-canvas + // mousedown default action is no longer prevented (see raycaster.js), so + // block drag-started text selection from anchoring on the canvas + user-select none + /* :where(:root) has zero specificity compared to :root, so any user defined :root will override the below rule */ :where(:root) { color-scheme: dark; @@ -301,9 +306,6 @@ body.aframe-inspector-opened .hide display none - .a-canvas.state-dragging - cursor grabbing - #rightPanel align-items stretch display flex @@ -325,10 +327,6 @@ body.aframe-inspector-opened #rightPanel pointer-events all - .aframe-inspector-opened a-scene .a-canvas - background-color #191919 - z-index 9998 - .toggle-sidebar align-items center display flex