diff --git a/.changeset/wild-parrots-train.md b/.changeset/wild-parrots-train.md new file mode 100644 index 0000000000..0fe2a9026e --- /dev/null +++ b/.changeset/wild-parrots-train.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Fix ActionMenu position issue when container is scrollable diff --git a/app/components/primer/alpha/action_menu.rb b/app/components/primer/alpha/action_menu.rb index b9c0e8cf8b..45b0f9f083 100644 --- a/app/components/primer/alpha/action_menu.rb +++ b/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", diff --git a/app/components/primer/alpha/action_menu/action_menu_element.ts b/app/components/primer/alpha/action_menu/action_menu_element.ts index 4793ba455d..08c6170f61 100644 --- a/app/components/primer/alpha/action_menu/action_menu_element.ts +++ b/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() { diff --git a/app/components/primer/alpha/select_panel_element.ts b/app/components/primer/alpha/select_panel_element.ts index 93cc438e36..5b7dd86dd1 100644 --- a/app/components/primer/alpha/select_panel_element.ts +++ b/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,7 +197,8 @@ export class SelectPanelElement extends HTMLElement { this.#softDisableItems() updateWhenVisible(this) - this.#waitForCondition( + observeMutationsUntilConditionMet( + this, () => Boolean(this.remoteInput), () => { this.remoteInput.addEventListener('loadstart', this, {signal}) @@ -204,7 +206,8 @@ export class SelectPanelElement extends HTMLElement { }, ) - 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() } diff --git a/app/components/primer/primer.ts b/app/components/primer/primer.ts index 3c7830831f..c10b45afa2 100644 --- a/app/components/primer/primer.ts +++ b/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' diff --git a/app/components/primer/utils.ts b/app/components/primer/utils.ts new file mode 100644 index 0000000000..ee7900c0ad --- /dev/null +++ b/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}) + } +} diff --git a/previews/primer/alpha/action_menu_preview/in_scroll_container.html.erb b/previews/primer/alpha/action_menu_preview/in_scroll_container.html.erb index 0f991715cd..b0d443165c 100644 --- a/previews/primer/alpha/action_menu_preview/in_scroll_container.html.erb +++ b/previews/primer/alpha/action_menu_preview/in_scroll_container.html.erb @@ -1,11 +1,9 @@ -
- -
+
+
<%= 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 %> +
- -