Implement keyboard support for single select_variant for TreeView and add some tests

Author: HDinger

app/components/primer/alpha/tree_view.pcss

@@ -197,8 +197,7 @@
       }
     }
 
-    &[aria-checked='false'],
-    &[aria-selected='false'] {
+    &[aria-checked='false'] {
       /* singleselect checkmark */
       & .TreeViewItem-singleSelectCheckmark {
         visibility: hidden;

app/components/primer/alpha/tree_view.rb

@@ -240,6 +240,14 @@ module Alpha
     #
     # Nodes can be checked via the keyboard by pressing the space key.
     #
+    # ## Single-select mode
+    #
+    # By passing `select_variant: :single` to both sub-tree and leaf nodes:
+    # - Nodes become selectable and can be toggled via keyboard (space key).
+    # - A selected node displays a checkmark at the end of the line.
+    # Note: This checkmark conflicts with the `trailing_visual_icon` slot,
+    # so both cannot be used simultaneously.
+    #
     # ## Node tags
     #
     # `TreeView`s support three different node variants, `:anchor`, `:button`, and `:div` (the default), which controls
@@ -259,13 +267,19 @@ module Alpha
     # |:---------------|:-------------|:------------|:--------------------------|
     # |Enter/space     |none          |div          |Expands/collapses          |
     # |Enter/space     |none          |anchor/button|Activates anchor/button    |
+    # |Enter/space     |single        |div          |Selects                    |
+    # |Enter/space     |single        |anchor/button|N/A (not allowed)          |
     # |Enter/space     |multiple      |div          |Checks or unchecks         |
     # |Enter/space     |multiple      |anchor/button|N/A (not allowed)          |
     # |Left/right arrow|none          |div          |Expands/collapses          |
     # |Left/right arrow|none          |anchor/button|Expands/collapses          |
+    # |Left/right arrow|single        |div          |Expands/collapses          |
+    # |Left/right arrow|single        |anchor/button|N/A (not allowed)          |
     # |Left/right arrow|multiple      |div          |Expands/collapses          |
     # |Left/right arrow|multiple      |anchor/button|N/A (not allowed)          |
     # |Click           |none          |div          |Expands/collapses          |
+    # |Click           |single        |div          |Selects                    |
+    # |Click           |single        |anchor/button|N/A (not allowed)          |
     # |Click           |multiple      |div          |Checks or unchecks         |
     # |Click           |multiple      |anchor/button|N/A (not allowed)          |
     #
@@ -351,7 +365,7 @@ module Alpha
     # )
     # ```
     #
-    # Because checking or unchecking a sub-tree results in the checking or unchecking of all its children recursively,
+    # Because checking or unchecking a sub-tree may result in the checking or unchecking of all its children recursively,
     # both the `treeViewNodeChecked` and `treeViewBeforeNodeChecked` events provide an array of `TreeViewNodeInfo`
     # objects, which contain entries for every modified node in the tree.
     class TreeView < Primer::Component

app/components/primer/alpha/tree_view/tree_view.ts

@@ -1,9 +1,7 @@
 import {controller, target} from '@github/catalyst'
-import {SelectStrategy, TreeViewSubTreeNodeElement} from './tree_view_sub_tree_node_element'
+import {SelectStrategy, SelectVariant, TreeViewSubTreeNodeElement} from './tree_view_sub_tree_node_element'
 import {useRovingTabIndex} from './tree_view_roving_tab_index'
-import type {TreeViewNodeType, TreeViewCheckedValue, TreeViewNodeInfo} from '../../shared_events'
-
-export type SelectVariant = 'none' | 'single' | 'multiple'
+import type {TreeViewCheckedValue, TreeViewNodeInfo, TreeViewNodeType} from '../../shared_events'
 
 @controller
 export class TreeViewElement extends HTMLElement {
@@ -126,6 +124,8 @@ export class TreeViewElement extends HTMLElement {
   #handleNodeEvent(node: Element, event: Event) {
     if (this.#eventIsCheckboxToggle(event, node)) {
       this.#handleCheckboxToggle(event, node)
+    } else if (this.#eventIsSingleSelection(event, node)) {
+      this.handleSingleSelection(event, node)
     } else if (this.#eventIsActivation(event)) {
       this.#handleNodeActivated(event, node)
     } else if (event.type === 'focusin') {
@@ -179,6 +179,48 @@ export class TreeViewElement extends HTMLElement {
     )
   }
 
+  #eventIsSingleSelection(event: Event, node: Element) {
+    return event.type === 'click' && this.selectVariant(node) === 'single'
+  }
+
+  handleSingleSelection(event: Event, node: Element) {
+    if (this.getNodeDisabledValue(node)) {
+      event.preventDefault()
+      return
+    }
+
+    // do not emit activation events for buttons and anchors, since it is assumed any activation
+    // behavior for these element types is user- or browser-defined
+    if (!(node instanceof HTMLDivElement)) return
+
+    const path = this.getNodePath(node)
+    const nodeInfo = this.infoFromNode(node, 'true')
+
+    const checkSuccess = this.dispatchEvent(
+      new CustomEvent('treeViewBeforeNodeChecked', {
+        bubbles: true,
+        cancelable: true,
+        detail: [nodeInfo],
+      }),
+    )
+
+    if (!checkSuccess) return
+
+    const currentlyChecked = !this.getNodeCheckedValue(node)
+
+    // disallow unchecking checked item in single-select mode
+    if (!currentlyChecked) {
+      this.checkOnlyAtPath(path)
+    }
+
+    this.dispatchEvent(
+      new CustomEvent('treeViewNodeChecked', {
+        bubbles: true,
+        detail: [this.infoFromNode(node, 'true')],
+      }),
+    )
+  }
+
   #handleNodeActivated(event: Event, node: Element) {
     if (this.getNodeDisabledValue(node)) {
       event.preventDefault()
@@ -201,19 +243,6 @@ export class TreeViewElement extends HTMLElement {
 
     if (!activationSuccess) return
 
-    if (this.selectVariant(node) === 'single') {
-      const currentlyChecked = !this.getNodeCheckedValue(node)
-
-      // disallow unchecking checked item in single-select mode
-      if (!currentlyChecked) {
-        for (const el of this.activeNodes) {
-          this.uncheckAtPath(this.getNodePath(el))
-        }
-
-        this.checkAtPath(path)
-      }
-    }
-
     // navigate or trigger button, don't toggle
     if (!this.nodeHasNativeAction(node)) {
       this.toggleAtPath(path)
@@ -246,14 +275,18 @@ export class TreeViewElement extends HTMLElement {
           break
         }
 
-        if (this.nodeHasCheckBox(node)) {
+        if (this.selectVariant(node) == 'multiple') {
           event.preventDefault()
 
           if (this.getNodeCheckedValue(node) === 'true') {
             this.setNodeCheckedValue(node, 'false')
           } else {
             this.setNodeCheckedValue(node, 'true')
           }
+        } else if (this.selectVariant(node) == 'single') {
+          event.preventDefault()
+
+          this.checkOnlyAtPath(this.getNodePath(node))
         } else if (node instanceof HTMLAnchorElement) {
           // simulate click on space
           node.click()
@@ -333,6 +366,14 @@ export class TreeViewElement extends HTMLElement {
     this.setNodeCheckedValue(node, 'false')
   }
 
+  checkOnlyAtPath(path: string[]) {
+    for (const el of this.activeNodes) {
+      this.uncheckAtPath(this.getNodePath(el))
+    }
+
+    this.checkAtPath(path)
+  }
+
   toggleCheckedAtPath(path: string[]) {
     const node = this.nodeAtPath(path)
     if (!node) return
@@ -440,7 +481,7 @@ export class TreeViewElement extends HTMLElement {
     }
   }
 
-  selectVariant(node: HTMLElement): SelectVariant {
+  selectVariant(node: Element): SelectVariant {
     return (node.getAttribute('data-select-variant') || 'none') as SelectVariant
   }
 }

app/components/primer/alpha/tree_view/tree_view_sub_tree_node_element.ts

@@ -9,6 +9,8 @@ type LoadingState = 'loading' | 'error' | 'success'
 
 export type SelectStrategy = 'self' | 'descendants' | 'mixed_descendants'
 
+export type SelectVariant = 'none' | 'single' | 'multiple'
+
 @controller
 export class TreeViewSubTreeNodeElement extends HTMLElement {
   @target node: HTMLElement
@@ -133,6 +135,10 @@ export class TreeViewSubTreeNodeElement extends HTMLElement {
     return (this.node.getAttribute('data-select-strategy') || 'descendants') as SelectStrategy
   }
 
+  get selectVariant(): SelectVariant {
+    return (this.node.getAttribute('data-select-variant') || 'none') as SelectVariant
+  }
+
   get level(): number {
     return parseInt(this.node.getAttribute('aria-level') || '0')
   }
@@ -321,6 +327,9 @@ export class TreeViewSubTreeNodeElement extends HTMLElement {
 
         if (this.#checkboxElement) {
           this.toggleChecked()
+        } else if (this.selectVariant == 'single') {
+          // Follow the standard implementation of TreeView and select that item
+          this.treeView.handleSingleSelection(event, node)
         } else if (!this.treeView?.nodeHasNativeAction(node)) {
           // toggle only if this node isn't eg. an anchor or button
           this.toggle()
@@ -352,6 +361,9 @@ export class TreeViewSubTreeNodeElement extends HTMLElement {
           event.preventDefault()
 
           this.toggleChecked()
+        } else if (this.selectVariant == 'single') {
+          // Follow the standard implementation of TreeView and select that item
+          this.treeView.handleSingleSelection(event, node)
         } else {
           if (node instanceof HTMLAnchorElement) {
             // simulate click on space for anchors (buttons already handle this natively)

previews/primer/alpha/tree_view_preview.rb

@@ -185,7 +185,7 @@ def auto_expansion
 
       # @label Form input
       #
-      # @param select_variant [Symbol] select [multiple, single, none]
+      # @param select_variant [Symbol] select [multiple, single]
       # @param expanded [Boolean] toggle
       def form_input(select_variant: :multiple, expanded: true)
         render_with_template(locals: {

previews/primer/alpha/tree_view_preview/default.html.erb

@@ -6,7 +6,7 @@
         <% icons.with_collapsed_icon(icon: :"file-directory-fill", color: :accent) %>
       <% end %>
 
-      <% sub_tree.with_trailing_visual_icon(icon: :"diff-modified") %>
+      <% sub_tree.with_trailing_visual_icon(icon: :"diff-modified") unless select_variant == :single %>
 
       <% sub_tree.with_leaf(label: "button.rb", disabled: disabled, select_variant: select_variant) do |item| %>
         <% item.with_leading_visual_icon(icon: :file) %>

test/components/primer/alpha/tree_view_test.rb

@@ -193,6 +193,19 @@ def test_falls_back_to_div_variant_when_invalid_variant_given
 
         assert_selector "div[role=treeitem]", text: "Foobar"
       end
+
+      def test_disallows_trailing_visuals_when_single_select_variant_is_used
+        error = assert_raises(ArgumentError) do
+          render_inline(Primer::Alpha::TreeView.new) do |tree|
+            tree.with_sub_tree(label: "src", select_variant: :single) do |sub_tree|
+              sub_tree.with_trailing_visual_icon(icon: :"diff-modified")
+              sub_tree.with_leaf(label: "button.rb")
+            end
+          end
+        end
+
+        assert_equal error.message, "Trailing visuals can't be used in combination with single select mode as the icon is reserved."
+      end
     end
   end
 end

test/system/alpha/tree_view_test.rb

@@ -607,6 +607,51 @@ def test_fires_activation_event
       assert_equal details["previousCheckedValue"], "false"
     end
 
+    def test_fires_event_before_checking_single_variant
+      visit_preview(:default, select_variant: :single)
+
+      details = capture_event("treeViewBeforeNodeChecked") do
+        activate_at_path("src")
+      end
+
+      assert_equal details.size, 1
+
+      assert details[0]["node"]
+      assert_equal details[0]["type"], "sub-tree"
+      assert_equal details[0]["path"], ["src"]
+      assert_equal details[0]["checkedValue"], "true"
+      assert_equal details[0]["previousCheckedValue"], "false"
+    end
+
+    def test_canceling_check_event_prevents_checking_for_single_variant
+      visit_preview(:default, select_variant: :single)
+
+      refute_path_checked "src"
+
+      capture_event("treeViewBeforeNodeChecked", cancel: true) do
+        activate_at_path("src")
+      end
+
+      # src should still not be checked
+      refute_path_checked "src"
+    end
+
+    def test_fires_check_event_after_single_variant
+      visit_preview(:default, select_variant: :single)
+
+      details = capture_event("treeViewNodeChecked") do
+        activate_at_path("src")
+      end
+
+      assert_equal details.size, 1
+
+      assert details[0]["node"]
+      assert_equal details[0]["type"], "sub-tree"
+      assert_equal details[0]["path"], ["src"]
+      assert_equal details[0]["checkedValue"], "true"
+      assert_equal details[0]["previousCheckedValue"], "false"
+    end
+
     def test_fires_event_before_checking
       visit_preview(:default, select_variant: :multiple)
 
@@ -786,7 +831,6 @@ def test_self_select_strategy_checking_sub_tree_does_not_check_children
       assert_path_checked "primer", "alpha", "action_bar"
     end
 
-
     def test_form_submission
       visit_preview(:form_input, expanded: true, route_format: :json)
 
@@ -799,5 +843,18 @@ def test_form_submission
 
       assert_equal "{\"path\":[\"action_menu.rb\"],\"value\":\"3\"}", response.dig("form_params", "folder_structure", 0)
     end
+
+    def test_form_submission_with_single_select_variant
+      visit_preview(:form_input, expanded: true, select_variant: :single, route_format: :json)
+
+      check_at_path("action_menu.rb")
+
+      find("button[type=submit]").click
+
+      # for some reason the JSON response is wrapped in HTML, I have no idea why
+      response = JSON.parse(find("pre").text)
+
+      assert_equal "{\"path\":[\"action_menu.rb\"],\"value\":\"3\"}", response.dig("form_params", "folder_structure", 0)
+    end
   end
 end