Do not populate dynamic label with active item in multi-select mode (#…

…3303)
primer · Jan 31, 2025 · 7fbd8da · 7fbd8da
1 parent a2af88f
commit 7fbd8da
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 3 deletions.
diff --git a/.changeset/stale-fishes-begin.md b/.changeset/stale-fishes-begin.md
@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Do not populate dynamic label with active item in multi-select mode
diff --git a/app/components/primer/alpha/action_menu/action_menu_element.ts b/app/components/primer/alpha/action_menu/action_menu_element.ts
@@ -143,6 +143,12 @@ export class ActionMenuElement extends HTMLElement {
       () => Boolean(this.invokerElement),
       () => this.#intersectionObserver.observe(this.invokerElement!),
     )
+
+    // If there's no include fragment, then no async fetching will occur and we can
+    // mark the component as ready.
+    if (!this.includeFragment) {
+      this.setAttribute('data-ready', 'true')
+    }
   }
 
   disconnectedCallback() {
@@ -367,6 +373,9 @@ export class ActionMenuElement extends HTMLElement {
   #handleIncludeFragmentReplaced() {
     if (this.#firstItem) this.#firstItem.focus()
     this.#softDisableItems()
+
+    // async items have loaded, so component is ready
+    this.setAttribute('data-ready', 'true')
   }
 
   // Close when focus leaves menu
@@ -387,6 +396,7 @@ export class ActionMenuElement extends HTMLElement {
   }
 
   #setDynamicLabel() {
+    if (this.selectVariant !== 'single') return
     if (!this.dynamicLabel) return
     const invokerLabel = this.invokerLabel
     if (!invokerLabel) return

diff --git a/previews/primer/alpha/action_menu_preview/multiple_select_form.html.erb b/previews/primer/alpha/action_menu_preview/multiple_select_form.html.erb
@@ -3,7 +3,7 @@
     <% menu.with_show_button { "Strategy" } %>
     <% menu.with_item(label: "Fast forward", data: { value: "fast_forward" }) %>
     <% menu.with_item(label: "Recursive", data: { value: "recursive" }) %>
-    <% menu.with_item(label: "Ours", data: { value: "ours" }) %>
+    <% menu.with_item(label: "Ours", data: { value: "ours" }, active: true) %>
     <% menu.with_item(label: "Resolve") %>
   <% end %>
   <hr>

diff --git a/test/system/alpha/action_menu_test.rb b/test/system/alpha/action_menu_test.rb
@@ -436,7 +436,9 @@ def test_multiple_select_form_submission
 
       # for some reason the JSON response is wrapped in HTML, I have no idea why
       response = JSON.parse(find("pre").text)
-      assert_equal %w[fast_forward recursive], response["value"]
+
+      # "ours" is pre-selected
+      assert_equal %w[fast_forward recursive ours], response["value"]
     end
 
     def test_multiple_select_form_uses_label_if_no_value_provided
@@ -453,7 +455,14 @@ def test_multiple_select_form_uses_label_if_no_value_provided
 
       # for some reason the JSON response is wrapped in HTML, I have no idea why
       response = JSON.parse(find("pre").text)
-      assert_equal %w[fast_forward Resolve], response["value"]
+
+      # "ours" is pre-selected
+      assert_equal %w[fast_forward ours Resolve], response["value"]
+    end
+
+    def test_multiple_select_does_not_set_dynamic_label_for_preselected_item
+      visit_preview(:multiple_select_form, route_format: :json)
+      assert_selector("action-menu[data-ready='true'] button[aria-controls]", exact_text: "Strategy")
     end
 
     def test_individual_items_can_submit_post_requests_via_forms