Skip to content
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

feat: auto-adjust menu width for custom triggers #7710

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/@react-aria/collections/src/Hidden.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {createPortal} from 'react-dom';
import {forwardRefType} from '@react-types/shared';
import React, {createContext, forwardRef, ReactElement, ReactNode, useContext} from 'react';
import React, {createContext, forwardRef, ReactElement, ReactNode, useContext, useMemo} from 'react';
import {useIsSSR} from '@react-aria/ssr';

// React doesn't understand the <template> element, which doesn't have children like a normal element.
Expand Down Expand Up @@ -43,6 +43,8 @@ const hiddenFragment = typeof DocumentFragment !== 'undefined' ? new DocumentFra
export function Hidden(props: {children: ReactNode}) {
let isHidden = useContext(HiddenContext);
let isSSR = useIsSSR();
// eslint-disable-next-line react-hooks/exhaustive-deps
let wasSSR = useMemo(() => isSSR, []);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was actually happening here? was strictmode running on the server or something?

Copy link
Contributor Author

@nwidynski nwidynski Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #7663 (comment)

TLDR; During hydration, useIsSSR() remains true, meaning we render into a template element on the first client render. The ref is attached but React can't maintain it as everything is moved to the portal in subsequent renders, in this case caused by the state update of the parent. This change forces a consistent strategy.

I tried using portal for the initial render as well as assigning both portal and template the same key, but to no avail also. I suppose this is due to bad support for template elements in general facebook/react#19932

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see, so now it will always render into template if it's an SSR app? we'll have to see if there's a performance hit for that
the reason for doing it in the portal was so we could use a super slimmed down document

if (isHidden) {
// Don't hide again if we are already hidden.
return <>{props.children}</>;
Expand All @@ -57,7 +59,7 @@ export function Hidden(props: {children: ReactNode}) {
// In SSR, portals are not supported by React. Instead, render into a <template>
// element, which the browser will never display to the user. In addition, the
// content is not part of the DOM tree, so it won't affect ids or other accessibility attributes.
return isSSR
return wasSSR
? <template data-react-aria-hidden>{children}</template>
: createPortal(children, hiddenFragment!);
}
Expand Down
26 changes: 24 additions & 2 deletions packages/react-aria-components/src/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@

import {AriaPopoverProps, DismissButton, Overlay, PlacementAxis, PositionProps, usePopover} from 'react-aria';
import {ContextValue, RenderProps, SlotProps, useContextProps, useRenderProps} from './utils';
import {filterDOMProps, mergeProps, useEnterAnimation, useExitAnimation, useLayoutEffect} from '@react-aria/utils';
import {filterDOMProps, mergeProps, useEnterAnimation, useExitAnimation, useLayoutEffect, useResizeObserver} from '@react-aria/utils';
import {forwardRefType, RefObject} from '@react-types/shared';
import {OverlayArrowContext} from './OverlayArrow';
import {OverlayTriggerProps, OverlayTriggerState, useOverlayTriggerState} from 'react-stately';
import {OverlayTriggerStateContext} from './Dialog';
import React, {createContext, ForwardedRef, forwardRef, useContext, useRef, useState} from 'react';
import React, {createContext, ForwardedRef, forwardRef, useCallback, useContext, useMemo, useRef, useState} from 'react';
import {useIsHidden} from '@react-aria/collections';

export interface PopoverProps extends Omit<PositionProps, 'isOpen'>, Omit<AriaPopoverProps, 'popoverRef' | 'triggerRef' | 'offset' | 'arrowSize'>, OverlayTriggerProps, RenderProps<PopoverRenderProps>, SlotProps {
Expand Down Expand Up @@ -91,6 +91,27 @@ export const Popover = /*#__PURE__*/ (forwardRef as forwardRefType)(function Pop
let isExiting = useExitAnimation(ref, state.isOpen) || props.isExiting || false;
let isHidden = useIsHidden();

// We can set minWidth to the trigger width as a courtesy for custom trigger elements.
let triggerWidth = Number.parseFloat(props.style?.['--trigger-width']);
let [menuWidth, setMenuWidth] = useState(triggerWidth || 0);

let onResize = useCallback(() => {
if (props.triggerRef?.current) {
let triggerRect = props.triggerRef.current.getBoundingClientRect();
setMenuWidth(triggerRect.right - triggerRect.left);
}
}, [props.triggerRef]);

useResizeObserver({
ref: props.triggerRef,
onResize: onResize
});

let style = useMemo(() => ({
minWidth: menuWidth + 'px',
...props.style
}), [menuWidth, props.style]);

// If we are in a hidden tree, we still need to preserve our children.
if (isHidden) {
let children = props.children;
Expand All @@ -114,6 +135,7 @@ export const Popover = /*#__PURE__*/ (forwardRef as forwardRefType)(function Pop
return (
<PopoverInner
{...props}
style={style}
triggerRef={props.triggerRef!}
state={state}
popoverRef={ref}
Expand Down
3 changes: 2 additions & 1 deletion packages/react-aria-components/test/ComboBox.ssr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ describe('ComboBox SSR', function () {
let [button, button2] = buttons;
fireEvent.click(button2);
expect(button2.textContent).toBe('1');
expect(button.textContent.split(', ')[2]).toBe(button.textContent.split(', ')[1]);
let [, first, second] = button.textContent.split(', ');
expect(first.replace(/ aria-describedby="[^"]*"/, '')).toBe(second);
});
});