fix: unfocus UI inputs on viewport click, refuse duplicate id in ID input (#860)

* fix: unfocus UI inputs on viewport click, refuse duplicate id in ID input

raycaster.js preventDefault'ed every mousedown on the viewport, which
canceled the browser's default focus transfer, so clicking the viewport
never unfocused the currently focused panel input. Only preventDefault
non-left buttons now (still needed so middle-click zoom doesn't trigger
autoscroll) and add user-select none on the canvas to keep drag-orbits
from starting a text selection, which the preventDefault was
incidentally providing.

changeId now refuses an id already used by another element (a duplicate
id breaks entity lookups done with getElementById/querySelector) and
shows a message below the input explaining the previous id was restored.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: remove dead a-canvas rules

The .aframe-inspector-opened a-scene .a-canvas and
.a-canvas.state-dragging rules are nested under #aframeInspector, and
the canvas is not a descendant of it, so they never matched. Nothing in
src adds the state-dragging class anyway.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: vincentfretin

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
             </label>
             <InputWidget
-              onBlur={changeId}
+              key={this.state.idInputKey}
+              onBlur={this.changeId}
               entity={entity}
               name="id"
               value={entity.id}
             />
           </div>
+          {this.state.duplicateId !== null && (
+            <div className="propertyRowError">
+              The id &quot;{this.state.duplicateId}&quot; is already used by
+              another element, the previous id was restored.
+            </div>
+          )}
           <div className="propertyRow">
             <label className="text">class</label>
             <span>{entity.getAttribute('class')}</span>

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,

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

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