[DREAM-752] Links in collapsible headers collapse/expand before redirecting

.changeset/orange-insects-rest.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Links in collapsible areas do not trigger collapse anymore

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/border_box/collapsible_header/collapsed/aria-snapshot.yml

@@ -1,2 +1,2 @@
-- button "Backlog":
-  - heading "Backlog" [level=3]
+- heading "Backlog" [level=3]
+- button "Backlog"

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/border_box/collapsible_header/default/aria-snapshot.yml

@@ -1,5 +1,5 @@
-- button "Backlog" [expanded]:
-  - heading "Backlog" [level=3]
+- heading "Backlog" [level=3]
+- button "Backlog" [expanded]
 - text: Body
 - list "Backlog":
   - listitem: Row 1

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/border_box/collapsible_header/with_count/aria-snapshot.yml

@@ -1,6 +1,6 @@
-- button "Backlog 42" [expanded]:
-  - heading "Backlog" [level=3]
-  - text: "42"
+- heading "Backlog" [level=3]
+- text: "42"
+- button "Backlog" [expanded]
 - text: Body
 - list "Backlog 42":
   - listitem: Row 1

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/border_box/collapsible_header/with_description/aria-snapshot.yml

@@ -1,7 +1,6 @@
-- button "Backlog This backlog is unique to this one-time meeting. You can drag items in and out to add or remove them from the meeting agenda." [expanded]:
-  - heading "Backlog" [level=3]
-  - text: This backlog is unique to this one-time meeting. You can drag items in and out to add or remove them from the meeting agenda.
-- text: Body
+- heading "Backlog" [level=3]
+- button "Backlog" [expanded]
+- text: This backlog is unique to this one-time meeting. You can drag items in and out to add or remove them from the meeting agenda. Body
 - list "Backlog This backlog is unique to this one-time meeting. You can drag items in and out to add or remove them from the meeting agenda.":
   - listitem: Row 1
   - listitem: Row 2

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/collapsible_section/collapsed/aria-snapshot.yml

@@ -1,2 +1,2 @@
-- button "Final question":
-  - heading "Final question" [level=2]
+- heading "Final question" [level=2]
+- button "Final question"

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/collapsible_section/default/aria-snapshot.yml

@@ -1,5 +1,5 @@
-- button "Feedback" [expanded]:
-  - heading "Feedback" [level=2]
+- heading "Feedback" [level=2]
+- button "Feedback" [expanded]
 - region "Feedback":
   - paragraph: Please take one minute time to answer this question
   - radiogroup "How did you hear about us?":

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/collapsible_section/with_additional_information/aria-snapshot.yml

@@ -1,6 +1,6 @@
-- button "Wednesday 11 6h 30min" [expanded]:
-  - heading "Wednesday 11" [level=2]
-  - text: 6h 30min
+- heading "Wednesday 11" [level=2]
+- button "Wednesday 11" [expanded]
+- text: 6h 30min
 - region "Wednesday 11 6h 30min":
   - text: Header Body
   - list "Header":

.playwright/screenshots/snapshots.test.ts-snapshots/primer/open_project/collapsible_section/with_caption/aria-snapshot.yml

@@ -1,6 +1,6 @@
-- button "Wednesday 11 Today" [expanded]:
-  - heading "Wednesday 11" [level=2]
-  - text: Today
+- heading "Wednesday 11" [level=2]
+- text: Today
+- button "Wednesday 11" [expanded]
 - region "Wednesday 11 Today":
   - text: Header Body
   - list "Header":

app/components/primer/open_project/border_box/collapsible_header.html.erb

@@ -5,7 +5,7 @@
       <% if count? %>
         <%= count %>
       <% end %>
-      <%= render(Primer::BaseComponent.new(tag: :div)) do %>
+      <%= render(Primer::Beta::Button.new(**@toggle_button_arguments)) do %>
         <%= render(Primer::Beta::Octicon.new(icon: "chevron-up", hidden: @collapsed, data: { target: "collapsible-header.arrowUp" })) %>
         <%= render(Primer::Beta::Octicon.new(icon: "chevron-down", hidden: !@collapsed, data: { target: "collapsible-header.arrowDown" })) %>
       <% end %>

app/components/primer/open_project/border_box/collapsible_header.rb

@@ -16,6 +16,7 @@ class CollapsibleHeader < Primer::Component
         # @param tag [Symbol] <%= one_of(Primer::OpenProject::BorderBox::CollapsibleHeader::TITLE_TAG_OPTIONS) %>
         # @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
         renders_one :title, lambda { |tag: TITLE_TAG_DEFAULT, **system_arguments|
+          system_arguments[:id] ||= @heading_id
           system_arguments[:classes] = class_names(
             system_arguments[:classes],
             "CollapsibleHeader-title",
@@ -29,7 +30,6 @@ class CollapsibleHeader < Primer::Component
         #
         # @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
         renders_one :count, lambda { |**system_arguments|
-          system_arguments[:mr] ||= 2
           system_arguments[:scheme] ||= :primary
           system_arguments[:classes] = class_names(
             system_arguments[:classes],
@@ -65,6 +65,7 @@ def initialize(id: self.class.generate_id, box: nil, collapsed: false, collapsib
 
           @collapsed = collapsed
           @collapsible_id = collapsible_id
+          @heading_id = "#{id}-heading"
 
           @system_arguments = deny_tag_argument(**system_arguments)
           @system_arguments[:tag] = :"collapsible-header"
@@ -87,16 +88,16 @@ def initialize(id: self.class.generate_id, box: nil, collapsed: false, collapsib
           end
 
           @trigger_area_arguments = { tag: :div }
-          @trigger_area_arguments[:role] = "button"
-          @trigger_area_arguments[:tabindex] = 0
           @trigger_area_arguments[:classes] = "CollapsibleHeader-triggerArea"
-          @trigger_area_arguments[:aria] = {
-            controls: @collapsible_id,
-            expanded: !@collapsed
-          }
           @trigger_area_arguments[:data] = {
-            target: "collapsible-header.triggerElement",
-            action: "click:collapsible-header#toggle keydown:collapsible-header#toggleViaKeyboard"
+            action: "click:collapsible-header#toggle"
+          }
+
+          @toggle_button_arguments = {
+            scheme: :invisible,
+            type: :button,
+            data: { target: "collapsible-header.triggerElement", "collapsible-toggle": "" },
+            aria: { expanded: !@collapsed, controls: @collapsible_id, labelledby: @heading_id }
           }
         end
 

app/components/primer/open_project/collapsible.ts

@@ -1,6 +1,5 @@
 import {attr, target, targets} from '@github/catalyst'
 
-// eslint-disable-next-line custom-elements/expose-class-on-global
 export abstract class CollapsibleElement extends HTMLElement {
   @target arrowDown: Element
   @target arrowUp: Element
@@ -11,12 +10,19 @@ export abstract class CollapsibleElement extends HTMLElement {
 
   toggleViaKeyboard(event: KeyboardEvent) {
     if (event.code === 'Enter' || event.code === 'Space') {
-      event.preventDefault()
-      this.toggle()
+      this.toggle(event)
     }
   }
 
-  toggle() {
+  toggle(event?: Event) {
+    if (
+      event &&
+      event.target instanceof Element &&
+      !event.target.closest('[data-collapsible-toggle]') &&
+      event.target.closest('a, button')
+    )
+      return
+    event?.preventDefault()
     this.collapsed = !this.collapsed
   }
 

app/components/primer/open_project/collapsible_section.html.erb

@@ -2,23 +2,19 @@
   <%= render(Primer::OpenProject::FlexLayout.new) do |flex| %>
     <%= flex.with_row(classes: "CollapsibleSection--triggerArea",
                       id: @title_id,
-                      tabindex: 0,
-                      role: "button",
-                      data: {
-                        action: "click:collapsible-section#toggle keydown:collapsible-section#toggleViaKeyboard",
-                        target: "collapsible-section.triggerElement"
-                      },
-                      aria: { expanded: !@collapsed, controls: @content_id }) do %>
+                      data: { action: "click:collapsible-section#toggle" }) do %>
       <%= render(Primer::OpenProject::FlexLayout.new(display: :flex, align_items: :center)) do |header| %>
-        <%= header.with_column do %>
+        <%= header.with_column(id: @heading_id) do %>
           <%= title %>
         <% end %>
         <%= header.with_column do %>
           <%= caption %>
         <% end %>
         <%= header.with_column do %>
-          <%= render(Primer::Beta::Octicon.new(icon: "chevron-up", hidden: @collapsed, data: { target: "collapsible-section.arrowUp" })) %>
-          <%= render(Primer::Beta::Octicon.new(icon: "chevron-down", hidden: !@collapsed, data: { target: "collapsible-section.arrowDown" })) %>
+          <%= render(Primer::Beta::Button.new(**@toggle_button_arguments)) do %>
+            <%= render(Primer::Beta::Octicon.new(icon: "chevron-up", hidden: @collapsed, data: { target: "collapsible-section.arrowUp" })) %>
+            <%= render(Primer::Beta::Octicon.new(icon: "chevron-down", hidden: !@collapsed, data: { target: "collapsible-section.arrowDown" })) %>
+          <% end %>
         <% end %>
         <%= header.with_column(flex: 1, text_align: :right) do %>
           <%= additional_information %>

app/components/primer/open_project/collapsible_section.rb

@@ -53,6 +53,7 @@ class CollapsibleSection < Primer::Component
       def initialize(id: self.class.generate_id, collapsed: false, **system_arguments)
         @title_id = "#{id}-title"
         @content_id = "#{id}-content"
+        @heading_id = "#{id}-heading"
         @collapsed = collapsed
 
         @system_arguments = deny_tag_argument(**system_arguments)
@@ -66,6 +67,13 @@ def initialize(id: self.class.generate_id, collapsed: false, **system_arguments)
 
         @system_arguments[:data] ||= {}
         @system_arguments[:data][:collapsed] = true if @collapsed
+
+        @toggle_button_arguments = {
+          scheme: :invisible,
+          type: :button,
+          data: { target: "collapsible-section.triggerElement", "collapsible-toggle": "" },
+          aria: { expanded: !@collapsed, controls: @content_id, labelledby: @heading_id }
+        }
       end
 
       private

previews/primer/open_project/collapsible_section_preview.rb

@@ -52,6 +52,15 @@ def collapsed
           locals: { }
         )
       end
+
+      # @label With link in title
+      # @hidden
+      def with_link_in_title
+        render_with_template(
+          template: "primer/open_project/collapsible_section_preview/with_link_in_title",
+          locals: { }
+        )
+      end
     end
   end
 end

previews/primer/open_project/collapsible_section_preview/with_link_in_title.html.erb

@@ -0,0 +1,6 @@
+<%= render(Primer::OpenProject::CollapsibleSection.new) do |section| %>
+  <% section.with_title { render(Primer::Beta::Link.new(href: "#")) { "Feedback" } } %>
+  <% section.with_collapsible_content do %>
+    <%= render(Primer::Alpha::Banner.new(mb: 3)) { "Please take one minute to answer this question" } %>
+  <% end %>
+<% end %>

static/info_arch.json

@@ -19682,6 +19682,19 @@
             "color-contrast"
           ]
         }
+      },
+      {
+        "preview_path": "primer/open_project/collapsible_section/with_link_in_title",
+        "name": "with_link_in_title",
+        "snapshot": "false",
+        "skip_rules": {
+          "wont_fix": [
+            "region"
+          ],
+          "will_fix": [
+            "color-contrast"
+          ]
+        }
       }
     ],
     "subcomponents": []

static/previews.json

@@ -8758,6 +8758,19 @@
             "color-contrast"
           ]
         }
+      },
+      {
+        "preview_path": "primer/open_project/collapsible_section/with_link_in_title",
+        "name": "with_link_in_title",
+        "snapshot": "false",
+        "skip_rules": {
+          "wont_fix": [
+            "region"
+          ],
+          "will_fix": [
+            "color-contrast"
+          ]
+        }
       }
     ]
   },

test/components/primer/open_project/border_box/collapsible_header_test.rb

@@ -91,15 +91,15 @@ def test_collapsible_id_sets_aria_controls
       header.with_title { "Backlog" }
     end
 
-    assert_selector(".CollapsibleHeader-triggerArea[aria-controls='body-id list-id']")
+    assert_selector(".CollapsibleHeader-triggerArea [data-collapsible-toggle][aria-controls='body-id list-id']")
   end
 
   def test_nil_collapsible_id_omits_aria_controls
     render_inline(Primer::OpenProject::BorderBox::CollapsibleHeader.new) do |header|
       header.with_title { "Backlog" }
     end
 
-    assert_no_selector(".CollapsibleHeader-triggerArea[aria-controls]")
+    assert_no_selector(".CollapsibleHeader-triggerArea [data-collapsible-toggle][aria-controls]")
   end
 
   def test_renders_with_description

test/system/open_project/border_box/collapsible_header_test.rb

@@ -30,14 +30,15 @@ def test_click_behaviour
     visit_preview(:default, module_prefix: "border_box")
 
     trigger = find(".CollapsibleHeader-triggerArea")
+    toggle_button = find(".CollapsibleHeader-triggerArea [data-collapsible-toggle]")
 
     # First, make sure it is not collapsed
     assert_no_selector(".CollapsibleHeader--collapsed")
     assert_selector(".octicon.octicon-chevron-down", visible: false)
     assert_selector(".octicon.octicon-chevron-up", visible: true)
 
     # Check aria-expanded is true when expanded
-    assert_equal "true", trigger[:'aria-expanded']
+    assert_equal "true", toggle_button[:'aria-expanded']
 
     # Collapse it
     trigger.click
@@ -46,7 +47,7 @@ def test_click_behaviour
     assert_selector(".octicon.octicon-chevron-up", visible: false)
     assert_selector(".octicon.octicon-chevron-down", visible: true)
 
-    assert_equal "false", trigger[:'aria-expanded']
+    assert_equal "false", toggle_button[:'aria-expanded']
 
     # Expand it again
     trigger.click
@@ -56,7 +57,7 @@ def test_click_behaviour
     assert_selector(".octicon.octicon-chevron-up", visible: true)
 
     # aria-expanded should be true again
-    trigger = find(".CollapsibleHeader-triggerArea")
-    assert_equal "true", trigger[:'aria-expanded']
+    toggle_button = find(".CollapsibleHeader-triggerArea [data-collapsible-toggle]")
+    assert_equal "true", toggle_button[:'aria-expanded']
   end
 end

test/system/open_project/collapsible_section_test.rb

@@ -21,14 +21,15 @@ def test_click_behaviour
     visit_preview(:default)
 
     trigger = find('.CollapsibleSection--triggerArea')
+    toggle_button = find('.CollapsibleSection--triggerArea [data-collapsible-toggle]')
 
     # Check aria-expanded is true when expanded
-    assert_equal "true", trigger[:'aria-expanded']
+    assert_equal "true", toggle_button[:'aria-expanded']
 
     # Check aria-controls is present and not empty
-    assert trigger[:'aria-controls'].present?, "Expected aria-controls attribute to be present"
+    assert toggle_button[:'aria-controls'].present?, "Expected aria-controls attribute to be present"
 
-    controlled_id = trigger[:'aria-controls']
+    controlled_id = toggle_button[:'aria-controls']
     assert_selector("##{controlled_id}", visible: true)
 
     # First, make sure it is not collapsed
@@ -46,7 +47,7 @@ def test_click_behaviour
     assert_no_text("How did you hear about us?")
 
     # aria-expanded should now be false
-    assert_equal "false", trigger[:'aria-expanded']
+    assert_equal "false", toggle_button[:'aria-expanded']
 
     # Controlled element should now be hidden
     assert_selector("##{controlled_id}", visible: false)
@@ -60,8 +61,27 @@ def test_click_behaviour
     assert_text("How did you hear about us?")
 
     # aria-expanded should be true again
-    trigger = find('.CollapsibleSection--triggerArea')
-    assert_equal "true", trigger[:'aria-expanded']
+    toggle_button = find('.CollapsibleSection--triggerArea [data-collapsible-toggle]')
+    assert_equal "true", toggle_button[:'aria-expanded']
     assert_selector("##{controlled_id}", visible: true)
   end
+
+  def test_link_in_title_does_not_collapse
+    visit_preview(:with_link_in_title)
+
+    assert_no_selector(".CollapsibleSection--collapsed")
+
+    # Clicking the trigger area (not the link) should still collapse
+    trigger = find(".CollapsibleSection--triggerArea")
+    trigger.click
+    assert_selector(".CollapsibleSection--collapsed")
+
+    # Expand again
+    trigger.click
+    assert_no_selector(".CollapsibleSection--collapsed")
+
+    # Clicking the link should NOT collapse the section
+    find(".CollapsibleSection--triggerArea a").click
+    assert_no_selector(".CollapsibleSection--collapsed")
+  end
 end