-
Notifications
You must be signed in to change notification settings - Fork 656
Dialog: dynamically switch footer button layout based on available height #7683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@primer/react': patch | ||
| --- | ||
|
|
||
| Dialog: dynamically switch footer button layout based on available height. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,7 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti | |||||||||||
| import classes from './Dialog.module.css' | ||||||||||||
| import {clsx} from 'clsx' | ||||||||||||
| import {useSlots} from '../hooks/useSlots' | ||||||||||||
| import {useResizeObserver} from '../hooks/useResizeObserver' | ||||||||||||
|
|
||||||||||||
| /* Dialog Version 2 */ | ||||||||||||
|
|
||||||||||||
|
|
@@ -239,6 +240,41 @@ const defaultPosition = { | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const defaultFooterButtons: Array<DialogButtonProps> = [] | ||||||||||||
| // Minimum room needed for body content before forcing footer buttons into horizontal scroll. | ||||||||||||
| const MIN_BODY_HEIGHT = 56 | ||||||||||||
|
|
||||||||||||
| // Measures what the footer height would be in wrap mode by cloning it offscreen, | ||||||||||||
| // so we can decide layout without mutating the visible footer. | ||||||||||||
| function measureWrappedFooterHeight(footerElement: HTMLElement) { | ||||||||||||
| const measurementContainer = document.createElement('div') | ||||||||||||
| const measuredFooter = footerElement.cloneNode(true) as HTMLElement | ||||||||||||
|
|
||||||||||||
| Object.assign(measurementContainer.style, { | ||||||||||||
| position: 'fixed', | ||||||||||||
| top: '0', | ||||||||||||
| left: '-99999px', | ||||||||||||
| visibility: 'hidden', | ||||||||||||
| pointerEvents: 'none', | ||||||||||||
| contain: 'layout style size', | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| measuredFooter.style.width = `${footerElement.getBoundingClientRect().width}px` | ||||||||||||
|
|
||||||||||||
| Object.assign(measuredFooter.style, { | ||||||||||||
| flexWrap: 'wrap', | ||||||||||||
| overflowX: '', | ||||||||||||
| overflowY: '', | ||||||||||||
| justifyContent: '', | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| measurementContainer.appendChild(measuredFooter) | ||||||||||||
| document.body.appendChild(measurementContainer) | ||||||||||||
|
|
||||||||||||
| const measuredHeight = measuredFooter.offsetHeight | ||||||||||||
| measurementContainer.remove() | ||||||||||||
|
|
||||||||||||
| return measuredHeight | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // useful to determine whether we're inside a Dialog from a nested component | ||||||||||||
| export const DialogContext = React.createContext<object | undefined>(undefined) | ||||||||||||
|
|
@@ -273,6 +309,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP | |||||||||||
| } | ||||||||||||
| } | ||||||||||||
| const [lastMouseDownIsBackdrop, setLastMouseDownIsBackdrop] = useState<boolean>(false) | ||||||||||||
| const [footerButtonLayout, setFooterButtonLayout] = useState<'scroll' | 'wrap'>('wrap') | ||||||||||||
| const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId} | ||||||||||||
| const onBackdropClick = useCallback( | ||||||||||||
| (e: SyntheticEvent) => { | ||||||||||||
|
|
@@ -339,6 +376,45 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP | |||||||||||
| const header = slots.header ?? (renderHeader ?? DefaultHeader)(defaultedProps) | ||||||||||||
| const body = slots.body ?? (renderBody ?? DefaultBody)({...defaultedProps, children: childrenWithoutSlots}) | ||||||||||||
| const footer = slots.footer ?? (renderFooter ?? DefaultFooter)(defaultedProps) | ||||||||||||
| const hasFooter = footer != null | ||||||||||||
|
|
||||||||||||
| const updateFooterButtonLayout = useCallback(() => { | ||||||||||||
| if (!hasFooter) { | ||||||||||||
| setFooterButtonLayout('wrap') | ||||||||||||
| return | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const dialogElement = dialogRef.current | ||||||||||||
| if (!(dialogElement instanceof HTMLElement)) { | ||||||||||||
| return | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const headerElement = dialogElement.querySelector(`.${classes.Header}`) | ||||||||||||
| const footerElement = dialogElement.querySelector(`.${classes.Footer}`) | ||||||||||||
|
|
||||||||||||
| if (!(footerElement instanceof HTMLElement)) { | ||||||||||||
| return | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const viewportHeight = backdropRef.current?.clientHeight ?? window.innerHeight | ||||||||||||
| const positionRegular = dialogElement.getAttribute('data-position-regular') | ||||||||||||
| const positionNarrow = dialogElement.getAttribute('data-position-narrow') | ||||||||||||
| // fullscreen/left/right fill the full viewport; otherwise match CSS max-height gutter. | ||||||||||||
| const gutter = viewportHeight <= 280 ? 12 : 64 | ||||||||||||
| const dialogMaxHeight = | ||||||||||||
| positionNarrow === 'fullscreen' || positionRegular === 'left' || positionRegular === 'right' | ||||||||||||
| ? viewportHeight | ||||||||||||
| : Math.max(0, viewportHeight - gutter) | ||||||||||||
|
|
||||||||||||
|
Comment on lines
+399
to
+408
|
||||||||||||
| const headerHeight = headerElement instanceof HTMLElement ? headerElement.offsetHeight : 0 | ||||||||||||
| const wrappedFooterHeight = measureWrappedFooterHeight(footerElement) | ||||||||||||
| const visibleBodyHeightWithWrap = Math.max(0, dialogMaxHeight - headerHeight - wrappedFooterHeight) | ||||||||||||
|
|
||||||||||||
| setFooterButtonLayout(visibleBodyHeightWithWrap >= MIN_BODY_HEIGHT ? 'wrap' : 'scroll') | ||||||||||||
| }, [hasFooter]) | ||||||||||||
|
|
||||||||||||
|
||||||||||||
| useEffect(() => { | |
| updateFooterButtonLayout() | |
| }, [updateFooterButtonLayout]) |
Copilot
AI
Mar 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR introduces a new data-footer-button-layout attribute that changes footer behavior based on measured heights, but Dialog.test.tsx doesn’t currently cover this new behavior. Adding a unit test that mocks ResizeObserver and element measurements (e.g. offsetHeight / getBoundingClientRect) would help prevent regressions for the wrap-vs-scroll switching logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
measureWrappedFooterHeightclones the entire footer, inserts it intodocument.body, forces layout, then removes it on every resize observer callback. This can be relatively expensive during viewport changes (especially on mobile where viewport height can change frequently). Consider caching/reusing a single measurement container and only re-measuring when width/footer content changes, to reduce DOM churn and forced reflow.