Improved keyboard UX and focus UI for Tree and OwnersStack

bvaughn · Apr 5, 2019 · d1a5346 · d1a5346
1 parent 632647f
commit d1a5346
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 13 deletions.
diff --git a/src/devtools/views/Components/OwnersStack.css b/src/devtools/views/Components/OwnersStack.css
@@ -14,6 +14,8 @@
   font-size: var(--font-size-monospace-normal);
   white-space: nowrap;
   border-radius: 0.125rem;
+  border: none;
+  background: none;
 }
 
 .Component {
@@ -23,11 +25,18 @@
 .Component:hover {
   background-color: var(--color-hover-background);
 }
+.Component:focus {
+  outline: none;
+  background-color: var(--color-hover-background);
+}
 
 .FocusedComponent {
   background-color: var(--color-selected-background);
   color: var(--color-selected-foreground);
 }
+.FocusedComponent:focus {
+  outline: none;
+}
 
 .VRule {
   height: 20px;

diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js
@@ -42,22 +42,20 @@ function ElementView({ id, index }: Props) {
   const store = useContext(StoreContext);
   const { displayName } = ((store.getElementByID(id): any): Element);
 
-  const isCurrentlyFocusedOwner = ownerStackIndex === index;
+  const isSelected = ownerStackIndex === index;
 
   const handleClick = useCallback(() => {
-    if (!isCurrentlyFocusedOwner) {
+    if (!isSelected) {
       selectOwner(id);
     }
-  }, [id, isCurrentlyFocusedOwner, selectOwner]);
+  }, [id, isSelected, selectOwner]);
 
   return (
-    <span
-      className={
-        isCurrentlyFocusedOwner ? styles.FocusedComponent : styles.Component
-      }
+    <button
+      className={isSelected ? styles.FocusedComponent : styles.Component}
       onClick={handleClick}
     >
       {displayName}
-    </span>
+    </button>
   );
 }
diff --git a/src/devtools/views/Components/Tree.css b/src/devtools/views/Components/Tree.css
@@ -18,11 +18,18 @@
   width: 100%;
   overflow: auto;
   flex: 1 0 auto;
-  padding: 0.25rem;
 }
 
 .List {
   font-family: var(--font-family-monospace);
   font-size: var(--font-size-monospace-normal);
   line-height: var(--line-height-data);
+  border: 0.25rem solid transparent;
+}
+.List:focus-within {
+  border-color: var(--color-button-background-focus);
+}
+
+.InnerElementType:focus {
+  outline: none;
 }
diff --git a/src/devtools/views/Components/Tree.js b/src/devtools/views/Components/Tree.js
@@ -1,6 +1,7 @@
 // @flow
 
 import React, {
+  useCallback,
   useContext,
   useEffect,
   useMemo,
@@ -132,8 +133,10 @@ export default function Tree(props: Props) {
 function InnerElementType({ style, ...rest }) {
   const {
     numElements,
+    selectedElementID,
     selectedElementIndex,
     selectElementAtIndex,
+    selectOwner,
   } = useContext(TreeContext);
 
   const handleFocus = () => {
@@ -142,20 +145,38 @@ function InnerElementType({ style, ...rest }) {
     }
   };
 
+  const handleKeyPress = useCallback(
+    event => {
+      switch (event.key) {
+        case 'Enter':
+        case ' ':
+          if (selectedElementID !== null) {
+            selectOwner(selectedElementID);
+          }
+          break;
+        default:
+          break;
+      }
+    },
+    [selectedElementID, selectOwner]
+  );
+
   // This style override enables the background color to fill the full visible width,
   // when combined with the CSS tweaks in Element.
   // A lot of options were considered; this seemed the one that requires the least code.
   // See https://github.com/bvaughn/react-devtools-experimental/issues/9
   return (
     <div
-      tabIndex={0}
+      className={styles.InnerElementType}
       onFocus={handleFocus}
+      onKeyPress={handleKeyPress}
       style={{
         ...style,
         display: 'inline-block',
         minWidth: '100%',
         width: undefined,
       }}
+      tabIndex={0}
       {...rest}
     />
   );

diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js
@@ -134,9 +134,11 @@ function reduceTreeState(store: Store, state: State, action: Action): State {
         break;
       case 'SELECT_NEXT_ELEMENT_IN_TREE':
         if (
-          selectedElementIndex !== null &&
-          selectedElementIndex + 1 < numElements
+          selectedElementIndex === null ||
+          selectedElementIndex + 1 >= numElements
         ) {
+          selectedElementIndex = 0;
+        } else {
           selectedElementIndex++;
         }
         break;
@@ -152,7 +154,9 @@ function reduceTreeState(store: Store, state: State, action: Action): State {
         }
         break;
       case 'SELECT_PREVIOUS_ELEMENT_IN_TREE':
-        if (selectedElementIndex !== null && selectedElementIndex > 0) {
+        if (selectedElementIndex === null || selectedElementIndex === 0) {
+          selectedElementIndex = numElements - 1;
+        } else {
           selectedElementIndex--;
         }
         break;