Merge pull request #20 from opf/upstream-update

Upstream update

.changeset/eighty-cheetahs-float.md

@@ -0,0 +1,7 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Support Rails edge's custom deprecators
+
+<!-- Changed components: _none_ -->

.changeset/plenty-experts-sort.md

@@ -0,0 +1,7 @@
+---
+'@openproject/primer-view-components': minor
+---
+
+Support SVGs as leading visuals in `Button` component
+
+<!-- Changed components: Primer::Beta::Button -->

.changeset/smart-scissors-type.md

@@ -0,0 +1,7 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Fix dialog invocation within deferred ActionMenus
+
+<!-- Changed components: Primer::Alpha::ActionMenu -->

.changeset/sweet-bananas-wave.md

@@ -0,0 +1,7 @@
+---
+"@openproject/primer-view-components": minor
+---
+
+Replace dismiss button in Flash with IconButton
+
+<!-- Changed components: Primer::Beta::Flash -->

.devcontainer/devcontainer.json

@@ -37,9 +37,6 @@
   "portsAttributes": {
     "4000": {
       "label": "demo"
-    },
-    "5400": {
-      "label": "doctocat"
     }
   },
   "hostRequirements": {

.github/workflows/demo-preview-destroy.yml

@@ -5,24 +5,34 @@ on:
     branches:
       - DO_NOT_EXECUTE_THIS_WORKFLOW
 
+permissions:
+  id-token: write # This is required for requesting the OIDC JWT for authing with Azure
+  contents: read  # This is required for actions/checkout
+
 # This allows one deploy workflow to interrupt another
 concurrency:
   group: 'preview-env @ ${{ github.head_ref || github.run_id }} for ${{ github.event.number || github.event.inputs.PR_NUMBER }}'
   cancel-in-progress: true
 
 jobs:
-  destory:
+  destroy:
     if: ${{ github.event.pull_request.head.repo.full_name == 'primer/view_components' && github.actor != 'dependabot[bot]' }}
     name: Destroy
     runs-on: ubuntu-latest
     timeout-minutes: 5
+    environment:
+      name: preview
     env:
       PR_NUMBER: ${{ github.event.number || github.event.inputs.PR_NUMBER }}
 
     steps:
       - uses: Azure/login@v1
         with:
-          creds: '{"clientId":"5ad1a188-b944-40eb-a2f8-cc683a6a65a0","clientSecret":"${{ secrets.AZURE_SPN_CLIENT_SECRET }}","subscriptionId":"550eb99d-d0c7-4651-a337-f53fa6520c4f","tenantId":"398a6654-997b-47e9-b12b-9515b896b4de"}'
+          # excluding a client secret here will cause a login via OpenID Connect (OIDC),
+          # which prevents us from having to rotate client credentials, etc
+          client-id: "5ad1a188-b944-40eb-a2f8-cc683a6a65a0"
+          tenant-id: "398a6654-997b-47e9-b12b-9515b896b4de"
+          subscription-id: "550eb99d-d0c7-4651-a337-f53fa6520c4f"
 
       - name: Check out repo
         uses: actions/checkout@v3

Gemfile

@@ -31,7 +31,7 @@ end
 # rubocop:enable Bundler/DuplicatedGem
 
 # Use Puma as the app server
-gem "puma", "~> 6.3.0"
+gem "puma", "~> 6.3.1"
 # Transpile app-like JavaScript. Read more: https://github.com/rails/webpacker
 gem "webpacker", "~> 5.0"
 

Gemfile.lock

@@ -128,7 +128,7 @@ GEM
       coderay (~> 1.1)
       method_source (~> 1.0)
     public_suffix (5.0.1)
-    puma (6.3.0)
+    puma (6.3.1)
       nio4r (~> 2.0)
     racc (1.6.2)
     rack (2.2.8)
@@ -255,7 +255,7 @@ DEPENDENCIES
   mocha
   openproject-primer_view_components!
   pry
-  puma (~> 6.3.0)
+  puma (~> 6.3.1)
   rack (~> 2.2)
   rack-cors
   railties (= 7.0.3)

app/components/primer/alpha/action_list.pcss

@@ -549,6 +549,12 @@
   grid-area: trailingAction;
 }
 
+/* have leading visual svg filled with chosen color */
+
+.ActionListItem-visual--leading svg {
+  fill: currentcolor;
+}
+
 /* wrapper span
  default block */
 .ActionListItem-descriptionWrap {

app/components/primer/alpha/action_menu/action_menu_element.ts

@@ -13,7 +13,8 @@ const menuItemSelectors = ['[role="menuitem"]', '[role="menuitemcheckbox"]', '[r
 
 @controller
 export class ActionMenuElement extends HTMLElement {
-  @target includeFragment: IncludeFragmentElement
+  @target
+  includeFragment: IncludeFragmentElement
 
   #abortController: AbortController
   #originalLabel = ''
@@ -57,7 +58,9 @@ export class ActionMenuElement extends HTMLElement {
     const id = this.querySelector('[role=menu]')?.id
     if (!id) return null
     for (const el of this.querySelectorAll(`[aria-controls]`)) {
-      if (el.getAttribute('aria-controls') === id) return el as HTMLButtonElement
+      if (el.getAttribute('aria-controls') === id) {
+        return el as HTMLButtonElement
+      }
     }
     return null
   }
@@ -94,7 +97,9 @@ export class ActionMenuElement extends HTMLElement {
     this.#updateInput()
 
     if (this.includeFragment) {
-      this.includeFragment.addEventListener('include-fragment-replaced', this, {signal})
+      this.includeFragment.addEventListener('include-fragment-replaced', this, {
+        signal
+      })
     }
   }
 
@@ -103,7 +108,8 @@ export class ActionMenuElement extends HTMLElement {
   }
 
   handleEvent(event: Event) {
-    if (event.target === this.invokerElement && this.#isActivationKeydown(event)) {
+    const activation = this.#isActivationKeydown(event)
+    if (event.target === this.invokerElement && activation) {
       if (this.#firstItem) {
         event.preventDefault()
         this.popoverElement?.showPopover()
@@ -112,11 +118,39 @@ export class ActionMenuElement extends HTMLElement {
       }
     }
 
+    // Ignore events within dialogs within menus
+    if ((event.target as Element)?.closest('dialog') || (event.target as Element)?.closest('modal-dialog')) {
+      return
+    }
+
+    // If a dialog has been rendered within the menu, we do not want to hide
+    // the entire menu, as that will also hide the Dialog. Instead we want to
+    // show the Dialog while hiding just the visible part of the menu.
+    if ((activation || event.type === 'click') && (event.target as HTMLElement)?.closest('[data-show-dialog-id]')) {
+      const dialogInvoker = (event.target as HTMLElement)!.closest('[data-show-dialog-id]')
+      const dialog = this.ownerDocument.getElementById(dialogInvoker?.getAttribute('data-show-dialog-id') || '')
+      if (dialogInvoker && dialog && this.contains(dialogInvoker) && this.contains(dialog)) {
+        this.querySelector<HTMLElement>('.ActionListWrap')!.style.display = 'none'
+        const dialog_controller = new AbortController()
+        const {signal} = dialog_controller
+        const handleDialogClose = () => {
+          dialog_controller.abort()
+          this.querySelector<HTMLElement>('.ActionListWrap')!.style.display = ''
+          if (this.popoverElement?.matches(':popover-open')) {
+            this.popoverElement?.hidePopover()
+          }
+        }
+        dialog.addEventListener('close', handleDialogClose, {signal})
+        dialog.addEventListener('cancel', handleDialogClose, {signal})
+        return
+      }
+    }
+
     if (!this.popoverElement?.matches(':popover-open')) return
 
     if (event.type === 'include-fragment-replaced') {
       if (this.#firstItem) this.#firstItem.focus()
-    } else if (this.#isActivationKeydown(event) || (event instanceof MouseEvent && event.type === 'click')) {
+    } else if (activation || (event instanceof MouseEvent && event.type === 'click')) {
       // Hide popover after current event loop to prevent changes in focus from
       // altering the target of the event. Not doing this specifically affects
       // <a> tags. It causes the event to be sent to the currently focused element

app/components/primer/alpha/dialog.rb

@@ -133,7 +133,8 @@ def initialize(
           @system_arguments, {
             aria: {
               disabled: true,
-              describedby: "#{@id}-title #{@id}-description"
+              labelledby: "#{@id}-title",
+              describedby: "#{@id}-description"
             }
           }
         )

app/components/primer/beta/button.pcss

@@ -110,6 +110,10 @@ summary.Button {
   grid-area: leadingVisual;
 }
 
+.Button-leadingVisual svg {
+  fill: currentcolor;
+}
+
 .Button-trailingVisual {
   grid-area: trailingVisual;
 }

app/components/primer/beta/button.rb

@@ -41,10 +41,15 @@ class Button < Primer::Component
       #
       # - `leading_visual_icon` for a <%= link_to_component(Primer::Beta::Octicon) %>.
       #
+      # - `leading_visual_svg` to render a SVG.
+      #
       # @param system_arguments [Hash] Same arguments as <%= link_to_component(Primer::Beta::Octicon) %>.
       renders_one :leading_visual, types: {
         icon: lambda { |**system_arguments|
           Primer::Beta::Octicon.new(**system_arguments)
+        },
+        svg: lambda { |**system_arguments|
+          Primer::BaseComponent.new(tag: :svg, width: "16", height: "16", **system_arguments)
         }
       }
 
@@ -81,7 +86,7 @@ class Button < Primer::Component
       renders_one :tooltip, lambda { |**system_arguments|
         raise ArgumentError, "Buttons with a tooltip must have a unique `id` set on the `Button`." if @id.blank? && !Rails.env.production?
 
-        ActiveSupport::Deprecation.warn("Buttons with visible text should not use a `label` tooltip. Consider using Primer::Beta::IconButton instead.") if system_arguments[:type] == :label
+        ::Primer::ViewComponents.deprecation.warn("Buttons with visible text should not use a `label` tooltip. Consider using Primer::Beta::IconButton instead.") if system_arguments[:type] == :label
         system_arguments[:for_id] = @id
         system_arguments[:type] = :description
 

app/components/primer/beta/flash.html.erb

@@ -1,9 +1,7 @@
 <%= render Primer::BaseComponent.new(**@system_arguments) do %>
   <%= primer_octicon @icon if @icon %><%= content %>
   <% if @dismissible %>
-    <button class="flash-close js-flash-close" type="button" aria-label="Close">
-      <%= primer_octicon "x" %>
-    </button>
+    <%= render Primer::Beta::IconButton.new(icon: :x, scheme: :invisible, wrapper_arguments: { classes: "flash-close" }, classes: "js-flash-close", "aria-label": "Dismiss alert") %>
   <% end %>
 
   <%= action %>

app/components/primer/beta/flash.pcss

@@ -31,8 +31,7 @@
 /* Close button */
 .flash-close:not(.Banner-close) {
   float: right;
-  padding: var(--control-medium-paddingInline-spacious);
-  margin: calc(var(--control-medium-paddingInline-spacious) * -1);
+  margin-top: calc(var(--base-size-4) * -1);
   text-align: center;
   cursor: pointer;
 

app/components/primer/component.rb

@@ -52,7 +52,7 @@ def deprecated_component_warning(new_class: nil, version: nil)
       message += " and will be removed in v#{version}." if version
       message += " Use #{new_class.name} instead." if new_class
 
-      ActiveSupport::Deprecation.warn(message)
+      ::Primer::ViewComponents.deprecation.warn(message)
     end
 
     def validate_aria_label

app/lib/primer/fetch_or_fallback_helper.rb

@@ -29,7 +29,7 @@ def fetch_or_fallback(allowed_values, given_value, fallback = nil, deprecated_va
       if allowed_values.include?(given_value)
         given_value
       elsif deprecated_values&.include?(given_value)
-        ActiveSupport::Deprecation.warn("#{given_value} is deprecated and will be removed in a future version.") unless Rails.env.production? || silence_deprecations?
+        ::Primer::ViewComponents.deprecation.warn("#{given_value} is deprecated and will be removed in a future version.") unless Rails.env.production? || silence_deprecations?
 
         given_value
       else

demo/Gemfile

@@ -31,7 +31,7 @@ end
 eval_gemfile "gemfiles/kuby.gemfile"
 
 # Use Puma as the app server
-gem "puma", "~> 6.3.0"
+gem "puma", "~> 6.3.1"
 
 # Reduces boot times through caching; required in config/boot.rb
 gem "bootsnap", ">= 1.4.2", require: false