Fix touch-enabled Chromium highlight on tree nodes

This commit resolves issues with the touch highlight behavior on tree nodes in touch-enabled Chromium browsers (such as Google Chrome). The fix addresses two issues: 1. Dual color transition issue during tapping actions on tree nodes. 2. Not highlighting full visible width of the node on keyboard focus. Other changes include: - Create `InteractableNode.vue` to centralize click styling and logic. - Remove redundant click/hover/touch styling from `LeafTreeNode.vue` and `HierarchicalTreeNode.vue`.
undergroundwires · Dec 15, 2023 · 2063397 · 2063397
1 parent 3457fe1
commit 2063397
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 87 deletions.
diff --git a/src/presentation/assets/styles/_mixins.scss b/src/presentation/assets/styles/_mixins.scss
@@ -1,34 +1,34 @@
 @mixin hover-or-touch($selector-suffix: '', $selector-prefix: '&') {
   @media (hover: hover) {
-
-    /*  We only do this if hover is truly supported; otherwise the emulator in mobile
-            keeps hovered style in-place even after touching, making it sticky.  */
+    /*
+      Only apply hover styles if the device truly supports hover; otherwise the
+      emulator in mobile keeps hovered style in-place even after touching, making it sticky.
+    */
     #{$selector-prefix}:hover #{$selector-suffix} {
       @content;
     }
   }
 
   @media (hover: none) {
-
-    /*  We only do this if hover is not supported,otherwise the desktop behavior is not
-            as desired; it does not get activated on hover but only during click/touch.  */
+    /*
+      Apply active styles on touch or click, ensuring interactive feedback on devices without hover capability.
+    */
     #{$selector-prefix}:active #{$selector-suffix} {
       @content;
     }
   }
 }
 
+/*
+  This mixin removes the default blue tap highlight seen in mobile WebKit browsers (e.g., Chrome, Safari, Edge).
+  The mixin by itself may reduce accessibility by hiding this interactive cue. Therefore, it is recommended
+  to use this mixin in conjunction with the `hover-or-touch` mixin to provide necessary visual feedback
+  for interactive elements during hover or touch interactions.
+*/
 @mixin clickable($cursor: 'pointer') {
   cursor: #{$cursor};
   user-select: none;
-  /*
-        It removes (blue) background during touch as seen in mobile webkit browsers (Chrome, Safari, Edge).
-        The default behavior is that any element (or containing element) that has cursor:pointer
-        explicitly set and is clicked will flash blue momentarily.
-        Removing it could have accessibility issue since that hides an interactive cue. But as we still provide
-        response to user actions through :active by `hover-or-touch` mixin.
-    */
-  -webkit-tap-highlight-color: transparent;
+  -webkit-tap-highlight-color: transparent; // Removes blue tap highlight
 }
 
 @mixin fade-transition($name) {

diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue b/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue
@@ -1,15 +1,17 @@
 <template>
   <div class="wrapper">
-    <div
+    <InteractableNode
       class="expansible-node"
       :style="{
         'padding-left': `${currentNode.hierarchy.depthInTree * 24}px`,
       }"
+      :node-id="nodeId"
+      :tree-root="treeRoot"
     >
       <div
         class="expand-collapse-arrow"
         :class="{
-          expanded: expanded,
+          expanded: isExpanded,
           'has-children': hasChildren,
         }"
         @click.stop="toggleExpand"
@@ -24,10 +26,10 @@
           </template>
         </LeafTreeNode>
       </div>
-    </div>
+    </InteractableNode>
     <transition name="children-transition">
       <ul
-        v-if="hasChildren && expanded"
+        v-if="hasChildren && isExpanded"
         class="children"
       >
         <HierarchicalTreeNode
@@ -54,12 +56,14 @@ import { NodeRenderingStrategy } from '../Rendering/Scheduling/NodeRenderingStra
 import { useNodeState } from './UseNodeState';
 import { TreeNode } from './TreeNode';
 import LeafTreeNode from './LeafTreeNode.vue';
+import InteractableNode from './InteractableNode.vue';
 import type { PropType } from 'vue';
 
 export default defineComponent({
   name: 'HierarchicalTreeNode', // Needed due to recursion
   components: {
     LeafTreeNode,
+    InteractableNode,
   },
   props: {
     nodeId: {
@@ -82,7 +86,7 @@ export default defineComponent({
     );
 
     const { state } = useNodeState(currentNode);
-    const expanded = computed<boolean>(() => state.value.isExpanded);
+    const isExpanded = computed<boolean>(() => state.value.isExpanded);
 
     const renderedNodeIds = computed<readonly string[]>(
       () => currentNode.value
@@ -96,18 +100,13 @@ export default defineComponent({
       currentNode.value.state.toggleExpand();
     }
 
-    function toggleCheck() {
-      currentNode.value.state.toggleCheck();
-    }
-
     const hasChildren = computed<boolean>(
       () => currentNode.value.hierarchy.isBranchNode,
     );
 
     return {
       renderedNodeIds,
-      expanded,
-      toggleCheck,
+      isExpanded,
       toggleExpand,
       currentNode,
       hasChildren,
@@ -123,7 +122,6 @@ export default defineComponent({
 .wrapper {
   display: flex;
   flex-direction: column;
-  cursor: pointer;
 
   .children {
     @include reset-ul;
@@ -140,16 +138,15 @@ export default defineComponent({
 
   flex-direction: row;
   align-items: center;
-  @include hover-or-touch {
-    background: $color-node-highlight-bg;
-  }
+
   .expand-collapse-arrow {
     flex-shrink: 0;
     height: 30px;
-    cursor: pointer;
     margin-left: 30px;
     width: 0;
 
+    @include clickable;
+
     &:after {
       position: absolute;
       display: block;

diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Node/InteractableNode.vue b/src/presentation/components/Scripts/View/Tree/TreeView/Node/InteractableNode.vue
@@ -0,0 +1,83 @@
+<template>
+  <div
+    class="clickable-node focusable-node"
+    tabindex="-1"
+    :class="{
+      'keyboard-focus': hasKeyboardFocus,
+    }"
+    @click.stop="toggleCheckState"
+    @focus="onNodeFocus"
+  >
+    <slot />
+  </div>
+</template>
+
+<script lang="ts">
+import { defineComponent, computed, toRef } from 'vue';
+import { TreeRoot } from '../TreeRoot/TreeRoot';
+import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
+import { useNodeState } from './UseNodeState';
+import { useKeyboardInteractionState } from './UseKeyboardInteractionState';
+import { TreeNode } from './TreeNode';
+import type { PropType } from 'vue';
+
+export default defineComponent({
+  props: {
+    nodeId: {
+      type: String,
+      required: true,
+    },
+    treeRoot: {
+      type: Object as PropType<TreeRoot>,
+      required: true,
+    },
+  },
+  setup(props) {
+    const { isKeyboardBeingUsed } = useKeyboardInteractionState();
+    const { nodes } = useCurrentTreeNodes(toRef(props, 'treeRoot'));
+    const currentNode = computed<TreeNode>(() => nodes.value.getNodeById(props.nodeId));
+    const { state } = useNodeState(currentNode);
+
+    const hasKeyboardFocus = computed<boolean>(() => {
+      if (!isKeyboardBeingUsed.value) {
+        return false;
+      }
+      return state.value.isFocused;
+    });
+
+    const onNodeFocus = () => {
+      props.treeRoot.focus.setSingleFocus(currentNode.value);
+    };
+
+    function toggleCheckState() {
+      currentNode.value.state.toggleCheck();
+    }
+
+    return {
+      onNodeFocus,
+      toggleCheckState,
+      currentNode,
+      hasKeyboardFocus,
+    };
+  },
+});
+</script>
+
+<style scoped lang="scss">
+@use "@/presentation/assets/styles/main" as *;
+@use "./../tree-colors" as *;
+
+.clickable-node {
+  @include clickable;
+  @include hover-or-touch {
+    background: $color-node-highlight-bg;
+  }
+}
+
+.focusable-node {
+  outline: none; // We handle keyboard focus through own styling
+  &.keyboard-focus {
+    background: $color-node-highlight-bg;
+  }
+}
+</style>
diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Node/LeafTreeNode.vue b/src/presentation/components/Scripts/View/Tree/TreeView/Node/LeafTreeNode.vue
@@ -1,43 +1,41 @@
 <template>
-  <li
-    class="node focusable"
-    tabindex="-1"
-    :class="{
-      'keyboard-focus': hasKeyboardFocus,
-    }"
-    @click.stop="toggleCheckState"
-    @focus="onNodeFocus"
-  >
-    <div class="node__layout">
-      <div class="node__checkbox">
-        <NodeCheckbox
-          :node-id="nodeId"
-          :tree-root="treeRoot"
-        />
+  <li>
+    <InteractableNode
+      :node-id="nodeId"
+      :tree-root="treeRoot"
+      class="node"
+    >
+      <div class="node__layout">
+        <div class="node__checkbox">
+          <NodeCheckbox
+            :node-id="nodeId"
+            :tree-root="treeRoot"
+          />
+        </div>
+        <div class="node__content content">
+          <slot
+            name="node-content"
+            :node-metadata="currentNode.metadata"
+          />
+        </div>
       </div>
-      <div class="node__content content">
-        <slot
-          name="node-content"
-          :node-metadata="currentNode.metadata"
-        />
-      </div>
-    </div>
+    </InteractableNode>
   </li>
 </template>
 
 <script lang="ts">
 import { defineComponent, computed, toRef } from 'vue';
 import { TreeRoot } from '../TreeRoot/TreeRoot';
 import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
-import { useNodeState } from './UseNodeState';
-import { useKeyboardInteractionState } from './UseKeyboardInteractionState';
 import { TreeNode } from './TreeNode';
 import NodeCheckbox from './NodeCheckbox.vue';
+import InteractableNode from './InteractableNode.vue';
 import type { PropType } from 'vue';
 
 export default defineComponent({
   components: {
     NodeCheckbox,
+    InteractableNode,
   },
   props: {
     nodeId: {
@@ -50,31 +48,11 @@ export default defineComponent({
     },
   },
   setup(props) {
-    const { isKeyboardBeingUsed } = useKeyboardInteractionState();
     const { nodes } = useCurrentTreeNodes(toRef(props, 'treeRoot'));
     const currentNode = computed<TreeNode>(() => nodes.value.getNodeById(props.nodeId));
-    const { state } = useNodeState(currentNode);
-
-    const hasKeyboardFocus = computed<boolean>(() => {
-      if (!isKeyboardBeingUsed.value) {
-        return false;
-      }
-      return state.value.isFocused;
-    });
-
-    const onNodeFocus = () => {
-      props.treeRoot.focus.setSingleFocus(currentNode.value);
-    };
-
-    function toggleCheckState() {
-      currentNode.value.state.toggleCheck();
-    }
 
     return {
-      onNodeFocus,
-      toggleCheckState,
       currentNode,
-      hasKeyboardFocus,
     };
   },
 });
@@ -97,27 +75,14 @@ export default defineComponent({
     overflow: auto; // Prevents horizontal expansion of inner content (e.g., when a code block is shown)
   }
 }
-
-.focusable {
-  outline: none; // We handle keyboard focus through own styling
-}
 .node {
   margin-bottom: 3px;
   margin-top: 3px;
   padding-bottom: 3px;
   padding-top: 3px;
   padding-right: 6px;
-  cursor: pointer;
   box-sizing: border-box;
 
-  &.keyboard-focus {
-    background: $color-node-highlight-bg;
-  }
-
-  @include hover-or-touch {
-    background: $color-node-highlight-bg;
-  }
-
   .content {
     display: flex; // We could provide `block`, but `flex` is more versatile.
     color: $color-node-fg;