Fix ActionMenu position issue when container is scrollable (#3207)

.changeset/wild-parrots-train.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Fix ActionMenu position issue when container is scrollable

app/components/primer/alpha/action_menu.rb

@@ -224,6 +224,12 @@ def initialize(
         @system_arguments[:"data-dynamic-label"] = "" if dynamic_label
         @system_arguments[:"data-dynamic-label-prefix"] = dynamic_label_prefix if dynamic_label_prefix.present?
 
+        overlay_arguments[:data] = merge_data(
+          overlay_arguments, data: {
+            target: "action-menu.overlay"
+          }
+        )
+
         @overlay = Primer::Alpha::Overlay.new(
           id: "#{@menu_id}-overlay",
           title: "Menu",

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

@@ -1,6 +1,8 @@
 import {controller, target} from '@github/catalyst'
 import '@oddbird/popover-polyfill'
 import type {IncludeFragmentElement} from '@github/include-fragment-element'
+import AnchoredPositionElement from '../../anchored_position'
+import {observeMutationsUntilConditionMet} from '../../utils'
 
 type SelectVariant = 'none' | 'single' | 'multiple' | null
 type SelectedItem = {
@@ -17,10 +19,14 @@ export class ActionMenuElement extends HTMLElement {
   @target
   includeFragment: IncludeFragmentElement
 
+  @target
+  overlay: AnchoredPositionElement
+
   #abortController: AbortController
   #originalLabel = ''
   #inputName = ''
   #invokerBeingClicked = false
+  #intersectionObserver: IntersectionObserver
 
   get selectVariant(): SelectVariant {
     return this.getAttribute('data-select-variant') as SelectVariant
@@ -106,6 +112,37 @@ export class ActionMenuElement extends HTMLElement {
         signal,
       })
     }
+
+    // The code below updates the menu (i.e. overlay) position whenever the invoker button
+    // changes position within its scroll container.
+    //
+    // See: https://github.com/primer/view_components/issues/3175
+
+    const scrollUpdater = () => {
+      if (this.#isOpen()) {
+        this.overlay?.update()
+      }
+    }
+
+    this.#intersectionObserver = new IntersectionObserver(entries => {
+      for (const entry of entries) {
+        const elem = entry.target
+        if (elem === this.invokerElement) {
+          if (entry.isIntersecting) {
+            // eslint-disable-next-line github/prefer-observers
+            window.addEventListener('scroll', scrollUpdater, {capture: true})
+          } else {
+            window.removeEventListener('scroll', scrollUpdater, {capture: true})
+          }
+        }
+      }
+    })
+
+    observeMutationsUntilConditionMet(
+      this,
+      () => Boolean(this.invokerElement),
+      () => this.#intersectionObserver.observe(this.invokerElement!),
+    )
   }
 
   disconnectedCallback() {

app/components/primer/alpha/select_panel_element.ts

@@ -6,6 +6,7 @@ import type {AnchorAlignment, AnchorSide} from '@primer/behaviors'
 import type {LiveRegionElement} from '@primer/live-region-element'
 import '@primer/live-region-element'
 import '@oddbird/popover-polyfill'
+import {observeMutationsUntilConditionMet} from '../utils'
 
 type SelectVariant = 'none' | 'single' | 'multiple' | null
 type SelectedItem = {
@@ -196,15 +197,17 @@ export class SelectPanelElement extends HTMLElement {
     this.#softDisableItems()
     updateWhenVisible(this)
 
-    this.#waitForCondition(
+    observeMutationsUntilConditionMet(
+      this,
       () => Boolean(this.remoteInput),
       () => {
         this.remoteInput.addEventListener('loadstart', this, {signal})
         this.remoteInput.addEventListener('loadend', this, {signal})
       },
     )
 
-    this.#waitForCondition(
+    observeMutationsUntilConditionMet(
+      this,
       () => Boolean(this.includeFragment),
       () => {
         this.includeFragment.addEventListener('include-fragment-replaced', this, {signal})
@@ -237,7 +240,8 @@ export class SelectPanelElement extends HTMLElement {
       }
     })
 
-    this.#waitForCondition(
+    observeMutationsUntilConditionMet(
+      this,
       () => Boolean(this.dialog),
       () => {
         this.#dialogIntersectionObserver.observe(this.dialog)
@@ -250,7 +254,8 @@ export class SelectPanelElement extends HTMLElement {
     )
 
     if (this.#fetchStrategy === FetchStrategy.LOCAL) {
-      this.#waitForCondition(
+      observeMutationsUntilConditionMet(
+        this,
         () => this.items.length > 0,
         () => {
           this.#updateItemVisibility()
@@ -260,23 +265,6 @@ export class SelectPanelElement extends HTMLElement {
     }
   }
 
-  // Waits for condition to return true. If it returns false initially, this function creates a
-  // MutationObserver that calls body() whenever the contents of the component change.
-  #waitForCondition(condition: () => boolean, body: () => void) {
-    if (condition()) {
-      body()
-    } else {
-      const mutationObserver = new MutationObserver(() => {
-        if (condition()) {
-          body()
-          mutationObserver.disconnect()
-        }
-      })
-
-      mutationObserver.observe(this, {childList: true, subtree: true})
-    }
-  }
-
   disconnectedCallback() {
     this.#abortController.abort()
   }

app/components/primer/primer.ts

@@ -8,6 +8,7 @@ import './dialog_helper'
 import './focus_group'
 import './scrollable_region'
 import './shared_events'
+import './utils'
 import './alpha/modal_dialog'
 import './beta/nav_list'
 import './beta/nav_list_group_element'

app/components/primer/utils.ts

@@ -0,0 +1,16 @@
+// Waits for condition to return true. If it returns false initially, this function creates a
+// MutationObserver that calls body() whenever the contents of the component change.
+export const observeMutationsUntilConditionMet = (element: Element, condition: () => boolean, body: () => void) => {
+  if (condition()) {
+    body()
+  } else {
+    const mutationObserver = new MutationObserver(() => {
+      if (condition()) {
+        body()
+        mutationObserver.disconnect()
+      }
+    })
+
+    mutationObserver.observe(element, {childList: true, subtree: true})
+  }
+}

previews/primer/alpha/action_menu_preview/in_scroll_container.html.erb

@@ -1,11 +1,9 @@
-<div style="height: 400px"></div>
-
-<div style="position: relative">
+<div style="height: 1000px; overflow: auto">
+  <div style="height: 400px"></div>
   <%= render Primer::Alpha::ActionMenu.new(anchor_align: :end) do |c| %>
     <% c.with_show_button { "Edit" } %>
     <% c.with_item(tag: :button, type: "button", label: "Rename") %>
     <% c.with_item(tag: :button, type: "button", scheme: :danger, label: "Remove") %>
   <% end %>
+  <div style="height: 1400px"></div>
 </div>
-
-<div style="height: 1400px"></div>