-
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
9e1de9c
c16f377
e3c38c7
2c08367
8eaebc1
7cfde85
9c10f79
00c9c2e
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,8 @@ const defaultPosition = { | |
| } | ||
|
|
||
| const defaultFooterButtons: Array<DialogButtonProps> = [] | ||
| // Minimum room needed for body content before forcing footer buttons into horizontal scroll. | ||
| const MIN_BODY_HEIGHT = 48 | ||
|
|
||
| // useful to determine whether we're inside a Dialog from a nested component | ||
| export const DialogContext = React.createContext<object | undefined>(undefined) | ||
|
|
@@ -273,6 +276,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 +343,35 @@ 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) { | ||
| return | ||
| } | ||
|
|
||
| const dialogElement = dialogRef.current | ||
| if (!(dialogElement instanceof HTMLElement)) { | ||
| return | ||
| } | ||
| const bodyWrapper = dialogElement.querySelector(`.${classes.DialogOverflowWrapper}`) | ||
| if (!(bodyWrapper instanceof HTMLElement)) { | ||
| return | ||
| } | ||
|
|
||
| // We temporarily force "wrap" the footer layout so that the browser can calculate the body height - | ||
| // when the footer is wrapping. This is instantaneous with what we set below (`dialogElement.setAttribute('data-footer-button-layout', newLayout)`). | ||
| dialogElement.setAttribute('data-footer-button-layout', 'wrap') | ||
| const bodyHeight = bodyWrapper.clientHeight | ||
|
|
||
| const newLayout = bodyHeight >= MIN_BODY_HEIGHT ? 'wrap' : 'scroll' | ||
| dialogElement.setAttribute('data-footer-button-layout', newLayout) | ||
|
|
||
| setFooterButtonLayout(newLayout) | ||
| }, [hasFooter]) | ||
|
|
||
| useResizeObserver(updateFooterButtonLayout, backdropRef) | ||
|
|
||
| const positionDataAttributes = | ||
| typeof position === 'string' | ||
| ? {'data-position-regular': position} | ||
|
|
@@ -371,7 +404,8 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP | |
| {...(align && {'data-align': align})} | ||
| data-width={width} | ||
| data-height={height} | ||
| data-has-footer={footer != null ? '' : undefined} | ||
| data-has-footer={hasFooter ? '' : undefined} | ||
| data-footer-button-layout={hasFooter ? footerButtonLayout : undefined} | ||
|
Comment on lines
+407
to
+408
|
||
| className={clsx(className, classes.Dialog)} | ||
| style={style} | ||
| > | ||
|
|
||
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.
updateFooterButtonLayoutis only triggered viauseResizeObserver(..., backdropRef), so the layout may become stale when footer/header content changes without a viewport/backdrop resize (e.g. button label changes, async loading states, togglingfooterButtons). Additionally, in theuseResizeObserverfallback path (noResizeObserver), the callback isn’t invoked on mount—only onwindow.resize—sodata-footer-button-layoutmay never be set correctly initially. Consider callingupdateFooterButtonLayout()in an effect when relevant inputs change (and/or observing the dialog/footer element too) so the layout decision is updated even without a resize event.