Merge pull request #188 from opf/bump/primer-upstream

Bump/primer upstream 0.34.0

.changeset/chilly-berries-retire.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+[SelectPanel] Disallow passing `role:` argument

.changeset/curly-phones-burn.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+[SelectPanel] Fix tab index issue in multi-select mode

.changeset/light-foxes-scream.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+SelectPanel - design bug fixes. Adds a divider below the filter, adjusts min height for loading and error states, moves banner error above input filter."

.changeset/many-zoos-scream.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+[SelectPanel] Raise an error when remote fetch + hidden filter argument combo

.changeset/old-starfishes-sell.md

@@ -0,0 +1,5 @@
+---
+"@openproject/primer-view-components": patch
+---
+
+`Label` padding updates to match PRC

.changeset/quiet-vans-unite.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': minor
+---
+
+Trigger an input event when the primer-text-field clear button is pressed.

.changeset/three-points-cross.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Don't select SelectPanel::Item when item is a link

.github/workflows/lint.yml

@@ -12,7 +12,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Get specific changed files
       id: changed-files
-      uses: tj-actions/[email protected].0
+      uses: tj-actions/[email protected].1
       with:
         files: |
           docs/**/*.md
@@ -39,7 +39,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Get specific changed files
       id: changed-files
-      uses: tj-actions/[email protected].0
+      uses: tj-actions/[email protected].1
       with:
         files: |
           app/**/*.rb
@@ -62,7 +62,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Get changed files
       id: changed-files
-      uses: tj-actions/[email protected].0
+      uses: tj-actions/[email protected].1
       with:
         files: |
           app/components/**/*.erb
@@ -82,7 +82,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Get changed files
       id: changed-files
-      uses: tj-actions/[email protected].0
+      uses: tj-actions/[email protected].1
       with:
         files: |
           app/components/**/*.ts
@@ -103,7 +103,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: Get changed files
       id: changed-files
-      uses: tj-actions/[email protected].0
+      uses: tj-actions/[email protected].1
       with:
         files: |
           app/components/**/*.pcss

.github/workflows/test.yml

@@ -70,32 +70,32 @@ jobs:
           - rails_version: "main"
             ruby_version: "3.3"
     steps:
-      - uses: actions/checkout@v4
-      - uses: ruby/setup-ruby@v1
-        with:
-          ruby-version: ${{ matrix.ruby_version }}
-          bundler-cache: true
-          cache-version: ${{ matrix.rails_version }}-${{ matrix.ruby_version }}
-      - uses: actions/setup-node@v4
-        with:
-          node-version: 16
-          cache: 'npm'
-      - name: Build
-        run: |
-          npm ci
-          bundle config unset deployment
-          bundle install --jobs 4 --retry 3
-        env:
-          RUBY_VERSION: ${{ matrix.ruby_version }}
-          RAILS_VERSION: ${{ matrix.rails_version }}
-      - name: Test
-        run: |
-          bundle exec rake test:components
-          bundle exec rake test:coverage
-        env:
-          COVERAGE: 1
-          RUBY_VERSION: ${{ matrix.ruby_version }}
-          RAILS_VERSION: ${{ matrix.rails_version }}
+    - uses: actions/checkout@v4
+    - uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: ${{ matrix.ruby_version }}
+        bundler-cache: true
+        cache-version: ${{ matrix.rails_version }}-${{ matrix.ruby_version }}
+    - uses: actions/setup-node@v4
+      with:
+        node-version: 16
+        cache: 'npm'
+    - name: Build
+      run: |
+        npm ci
+        bundle config unset deployment
+        bundle install --jobs 4 --retry 3
+      env:
+        RUBY_VERSION: ${{ matrix.ruby_version }}
+        RAILS_VERSION: ${{ matrix.rails_version }}
+    - name: Test
+      run: |
+        bundle exec rake test:components
+        bundle exec rake test:coverage
+      env:
+        COVERAGE: 1
+        RUBY_VERSION: ${{ matrix.ruby_version }}
+        RAILS_VERSION: ${{ matrix.rails_version }}
   lib:
     name: Lib
     runs-on:
@@ -122,32 +122,32 @@ jobs:
           - rails_version: "main"
             ruby_version: "3.3"
     steps:
-      - uses: actions/checkout@v4
-      - uses: ruby/setup-ruby@v1
-        with:
-          ruby-version: ${{ matrix.ruby_version }}
-          bundler-cache: true
-          cache-version: ${{ matrix.rails_version }}-${{ matrix.ruby_version }}
-      - uses: actions/setup-node@v4
-        with:
-          node-version: 16
-          cache: 'npm'
-      - name: Build
-        run: |
-          npm ci
-          bundle config unset deployment
-          bundle install --jobs 4 --retry 3
-        env:
-          RUBY_VERSION: ${{ matrix.ruby_version }}
-          RAILS_VERSION: ${{ matrix.rails_version }}
-      - name: Test
-        run: |
-          bundle exec rake test:lib
-          bundle exec rake test:coverage
-        env:
-          COVERAGE: 1
-          RUBY_VERSION: ${{ matrix.ruby_version }}
-          RAILS_VERSION: ${{ matrix.rails_version }}
+    - uses: actions/checkout@v4
+    - uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: ${{ matrix.ruby_version }}
+        bundler-cache: true
+        cache-version: ${{ matrix.rails_version }}-${{ matrix.ruby_version }}
+    - uses: actions/setup-node@v4
+      with:
+        node-version: 16
+        cache: 'npm'
+    - name: Build
+      run: |
+        npm ci
+        bundle config unset deployment
+        bundle install --jobs 4 --retry 3
+      env:
+        RUBY_VERSION: ${{ matrix.ruby_version }}
+        RAILS_VERSION: ${{ matrix.rails_version }}
+    - name: Test
+      run: |
+        bundle exec rake test:lib
+        bundle exec rake test:coverage
+      env:
+        COVERAGE: 1
+        RUBY_VERSION: ${{ matrix.ruby_version }}
+        RAILS_VERSION: ${{ matrix.rails_version }}
   performance:
     name: Performance
     runs-on: ubuntu-latest

app/components/primer/alpha/select_panel.html.erb

@@ -2,14 +2,23 @@
   <dialog-helper>
     <%= show_button %>
     <%= render(@dialog) do %>
-      <%= render Primer::Alpha::Dialog::Header.new(id: "#{@panel_id}-dialog", title: @title) do |header| %>
+      <%= render Primer::Alpha::Dialog::Header.new(id: "#{@panel_id}-dialog", title: @title, show_divider: show_filter?) do |header| %>
         <% if subtitle? %>
           <% header.with_subtitle do %>
             <%= subtitle %>
           <% end %>
         <% end %>
         <% if show_filter? %>
           <% header.with_filter do %>
+            <div data-target="select-panel.bannerErrorElement" hidden>
+              <%= render Primer::Alpha::Banner.new(scheme: :danger, mb: 2) do %>
+                <% if error_content? %>
+                  <%= error_content %>
+                <% else %>
+                  <h2 class="f6 text-normal">Sorry, something went wrong.</h2>
+                <% end %>
+              <% end %>
+            </div>
             <%= render(Primer::BaseComponent.new(
               tag: :"remote-input",
               aria: { owns: @body_id },
@@ -36,16 +45,7 @@
           <% end %>
         <% end %>
       <% end %>
-      <div data-target="select-panel.bannerErrorElement" hidden>
-        <%= render Primer::Alpha::Banner.new(scheme: :danger, full: true, mt: 2) do %>
-          <% if error_content? %>
-            <%= error_content %>
-          <% else %>
-            <h2 class="f5">Sorry, something went wrong.</h2>
-          <% end %>
-        <% end %>
-      </div>
-      <%= render Primer::Alpha::Dialog::Body.new(mt: 2, p: 0) do %>
+      <%= render Primer::Alpha::Dialog::Body.new(mt: show_filter? ? 0 : 2, p: 0) do %>
         <focus-group direction="vertical" mnemonics retain>
           <div class="sr-only" aria-live="polite" aria-atomic="true" data-target="select-panel.ariaLiveContainer"></div>
           <%= render(Primer::BaseComponent.new(
@@ -67,7 +67,8 @@
                     display: :flex,
                     align_items: :center,
                     justify_content: :center,
-                    text_align: :center
+                    text_align: :center,
+                    classes: "SelectPanel-loadingPanel"
                   )) do %>
                     <div data-hide-on-error>
                       <%= render Primer::Beta::Spinner.new(aria: { label: "Loading content..." }, data: { target: "select-panel.bodySpinner" }) %>
@@ -78,7 +79,7 @@
                       <% else %>
                         <div class="pt-2 pb-2">
                           <%= render Primer::Beta::Octicon.new(icon: :alert, color: :danger) %>
-                          <h2 class="f5">Sorry, something went wrong.</h2>
+                          <h2 class="f5 mt-2">Sorry, something went wrong.</h2>
                         </div>
                       <% end %>
                     </div>
@@ -88,7 +89,7 @@
                 <%= render(@list) %>
               <% end %>
             </div>
-            <div data-target="select-panel.noResults" class="color-border-muted text-center" hidden>
+            <div data-target="select-panel.noResults" class="color-border-muted text-center d-flex flex-items-center flex-justify-center SelectPanel-emptyPanel" hidden>
               <h2 class="v-align-middle m-3 f5"><%= @no_results_label %></h2>
             </div>
           <% end %>

app/components/primer/alpha/select_panel.pcss

@@ -0,0 +1,7 @@
+.SelectPanel-loadingPanel {
+  min-height: min(var(--overlay-height) - 10rem, 100vh - 2rem);
+}
+
+.SelectPanel-emptyPanel {
+  min-height: min(var(--overlay-height) - 10rem, 100vh - 2rem);
+}

app/components/primer/alpha/select_panel.rb

@@ -250,10 +250,28 @@ module Alpha
     # )
     # ```
     class SelectPanel < Primer::Component
+      # @private
+      module Utils
+        def raise_if_role_given!(**system_arguments)
+          return if shouldnt_raise_error?
+          return unless system_arguments.include?(:role)
+
+          raise(
+            "Please avoid passing the `role:` argument to `SelectPanel` and its subcomponents. "\
+            "The component will automatically apply the correct roles where necessary."
+          )
+        end
+      end
+
+      include Utils
+
       # The component that should be used to render the list of items in the body of a SelectPanel.
       class ItemList < Primer::Alpha::ActionList
+        include Utils
+
         # @param system_arguments [Hash] The arguments accepted by <%= link_to_component(Primer::Alpha::ActionList) %>.
         def initialize(**system_arguments)
+          raise_if_role_given!(**system_arguments)
           select_variant = system_arguments[:select_variant] || Primer::Alpha::ActionList::DEFAULT_SELECT_VARIANT
 
           super(
@@ -263,6 +281,16 @@ def initialize(**system_arguments)
             **system_arguments
           )
         end
+
+        def with_item(**system_arguments)
+          raise_if_role_given!(**system_arguments)
+          super
+        end
+
+        def with_avatar_item(**system_arguments)
+          raise_if_role_given!(**system_arguments)
+          super
+        end
       end
 
       status :alpha
@@ -362,6 +390,8 @@ def initialize(
         anchor_side: Primer::Alpha::Overlay::DEFAULT_ANCHOR_SIDE,
         **system_arguments
       )
+        raise_if_role_given!(**system_arguments)
+
         if src.present?
           url = URI(src)
           query = url.query || ""
@@ -419,12 +449,21 @@ def initialize(
           form_arguments: form_arguments,
           id: "#{@panel_id}-list",
           select_variant: @select_variant,
-          role: "listbox",
-          aria_selection_variant: @select_variant == :multiple ? :checked : :selected,
           aria: {
             label: "#{title} options"
-          },
-          p: 2
+          }
+        )
+
+        return if @show_filter || @fetch_strategy != :remote
+        return if shouldnt_raise_error?
+
+        raise(
+          "Hiding the filter input with a remote fetch strategy is not permitted, "\
+          "since such a combinaton of options will cause the component to only "\
+          "fetch items from the server once when the panel opens for the first time; "\
+          "this is what the `:eventually_local` fetch strategy is designed to do. "\
+          "Consider passing `show_filter: true` or use the `:eventually_local` fetch "\
+          "strategy instead."
         )
       end
 

app/components/primer/alpha/select_panel_element.ts

@@ -314,11 +314,7 @@ export class SelectPanelElement extends HTMLElement {
         const itemContent = this.#getItemContent(item)
         if (!itemContent) continue
 
-        if (!this.isItemHidden(item) && !setZeroTabIndex) {
-          setZeroTabIndex = true
-        } else {
-          itemContent.setAttribute('tabindex', '-1')
-        }
+        itemContent.setAttribute('tabindex', '-1')
 
         // <li> elements should not themselves be tabbable
         item.removeAttribute('tabindex')
@@ -742,6 +738,8 @@ export class SelectPanelElement extends HTMLElement {
       return true
     }
 
+    if (!this.bannerErrorElement) return false
+
     return !this.bannerErrorElement.hasAttribute('hidden')
   }
 
@@ -882,6 +880,9 @@ export class SelectPanelElement extends HTMLElement {
     const itemContent = this.#getItemContent(item)
 
     if (this.selectVariant === 'single') {
+      // Don't check anything if we have an href
+      if (itemContent?.getAttribute('href')) return
+
       // disallow unchecking checked item in single-select mode
       if (!currentlyChecked) {
         for (const el of this.items) {