Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 677
SelectPanel2: Use html dialog#4020
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
f3eddf6
copy changes from #4018
siddharthkp fb0c11f
Merge branch 'main' into drafts-selectpanel-html-dialog
siddharthkp 262316c
remove undefined values
siddharthkp 907c1a0
add autofocus
siddharthkp 56277f0
sync esc with internalClose
siddharthkp 4f5e5bd
move focus logic to only work once
siddharthkp 613ef0f
change tooltip direction to stay within input
siddharthkp ca8f8b6
note for self
siddharthkp 19bc492
add temporary example for question
siddharthkp 5de2729
Revert "add temporary example for question"
siddharthkp 2d68105
move comment closer to code
siddharthkp 529639a
nudge user towards actions when clicking outside
siddharthkp 635ad1a
oops
siddharthkp dfcb85c
Create eleven-lizards-draw.md
siddharthkp a8430d4
change animation duration to 350ms
siddharthkp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@primer/react": patch | ||
| --- | ||
| experimental/SelectPanel2: Use `<dialog>` element |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,8 +8,6 @@ import { | ||
| IconButton, | ||
| Heading, | ||
| Box, | ||
| AnchoredOverlay, | ||
| AnchoredOverlayProps, | ||
| Tooltip, | ||
| TextInput, | ||
| TextInputProps, | ||
| @@ -20,8 +18,9 @@ import { | ||
| } from '../../../src/index' | ||
| import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' | ||
| import {useSlots} from '../../hooks/useSlots' | ||
| import {useProvidedRefOrCreate, useId} from '../../hooks' | ||
| import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks' | ||
| import {useFocusZone} from '../../hooks/useFocusZone' | ||
| import {StyledOverlay, OverlayProps} from '../../Overlay/Overlay' | ||
| const SelectPanelContext = React.createContext<{ | ||
| title: string | ||
| @@ -58,8 +57,8 @@ export type SelectPanelProps = { | ||
| onSubmit?: (event?: React.FormEvent<HTMLFormElement>) => void | ||
| // TODO: move these to SelectPanel.Overlay or overlayProps | ||
| width?: AnchoredOverlayProps['width'] | ||
| height?: AnchoredOverlayProps['height'] | ||
| width?: OverlayProps['width'] | ||
| height?: OverlayProps['height'] | ||
| children: React.ReactNode | ||
| } | ||
| @@ -82,24 +81,38 @@ const Panel: React.FC<SelectPanelProps> = ({ | ||
| height = 'large', | ||
| ...props | ||
| }) => { | ||
| const anchorRef = useProvidedRefOrCreate(providedAnchorRef) | ||
| const [internalOpen, setInternalOpen] = React.useState(defaultOpen) | ||
| // sync open state with props | ||
| if (propsOpen !== undefined && internalOpen !== propsOpen) setInternalOpen(propsOpen) | ||
| // TODO: replace this hack with clone element? | ||
| // 🚨 Hack for good API! | ||
| // we strip out Anchor from children and pass it to AnchoredOverlay to render | ||
| // we strip out Anchor from children and wire it up to Dialog | ||
| // with additional props for accessibility | ||
| let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null | ||
| let Anchor: React.ReactElement | undefined | ||
| const anchorRef = useProvidedRefOrCreate(providedAnchorRef) | ||
| const onAnchorClick = () => { | ||
| if (!internalOpen) setInternalOpen(true) | ||
| else onInternalClose() | ||
| } | ||
| const contents = React.Children.map(props.children, child => { | ||
| if (React.isValidElement(child) && child.type === SelectPanelButton) { | ||
| renderAnchor = anchorProps => React.cloneElement(child, anchorProps) | ||
| Anchor = React.cloneElement(child, { | ||
| // @ts-ignore TODO | ||
| ref: anchorRef, | ||
| onClick: onAnchorClick, | ||
| 'aria-haspopup': true, | ||
| 'aria-expanded': internalOpen, | ||
| }) | ||
| return null | ||
| } | ||
| return child | ||
| }) | ||
| const [internalOpen, setInternalOpen] = React.useState(defaultOpen) | ||
| // sync open state | ||
| if (propsOpen !== undefined && internalOpen !== propsOpen) setInternalOpen(propsOpen) | ||
| const onInternalClose = () => { | ||
| if (propsOpen === undefined) setInternalOpen(false) | ||
| if (typeof propsOnCancel === 'function') propsOnCancel() | ||
| @@ -135,26 +148,77 @@ const Panel: React.FC<SelectPanelProps> = ({ | ||
| [internalOpen], | ||
| ) | ||
| /* Dialog */ | ||
| const dialogRef = React.useRef<HTMLDialogElement>(null) | ||
| if (internalOpen) dialogRef.current?.showModal() | ||
| else dialogRef.current?.close() | ||
| // dialog handles Esc automatically, so we have to sync internal state | ||
| React.useEffect(() => dialogRef.current?.addEventListener('close', onInternalClose)) | ||
| // React doesn't support autoFocus for dialog: https://github.com/facebook/react/issues/23301 | ||
| // tl;dr: react takes over autofocus instead of letting the browser handle it, | ||
| // but not for dialogs, so we have to do it | ||
| React.useEffect(() => { | ||
| if (internalOpen) document.querySelector('input')?.focus() | ||
| }, [internalOpen]) | ||
| /* Anchored */ | ||
| const {position} = useAnchoredPosition( | ||
| { | ||
| anchorElementRef: anchorRef, | ||
| floatingElementRef: dialogRef, | ||
| side: 'outside-bottom', | ||
| align: 'start', | ||
| }, | ||
| [anchorRef.current, dialogRef.current], | ||
| ) | ||
| /* | ||
| We don't close the panel when clicking outside. | ||
| For many years, we used to save changes and closed the dialog (for label picker) | ||
| which isn't accessible, clicking outside should discard changes and close the dialog | ||
| Fixing this a11y bug would confuse users, so as a middle ground, | ||
| we don't close the menu and nudge the user towards the footer actions | ||
| */ | ||
| const [footerAnimationEnabled, setFooterAnimationEnabled] = React.useState(false) | ||
| const onClickOutside = () => { | ||
| setFooterAnimationEnabled(true) | ||
| window.setTimeout(() => setFooterAnimationEnabled(false), 350) | ||
| } | ||
| return ( | ||
| <> | ||
| <AnchoredOverlay | ||
| anchorRef={anchorRef} | ||
| renderAnchor={renderAnchor} | ||
| open={internalOpen} | ||
| onOpen={() => setInternalOpen(true)} | ||
| onClose={onInternalClose} | ||
| {Anchor} | ||
| <StyledOverlay | ||
| as="dialog" | ||
| ref={dialogRef} | ||
| aria-labelledby={`${panelId}--title`} | ||
| aria-describedby={description ? `${panelId}--description` : undefined} | ||
| width={width} | ||
| height={height} | ||
| focusZoneSettings={{ | ||
| // we only want focus trap from the overlay, | ||
| // we don't want focus zone on the whole overlay because | ||
| // we have a focus zone on the list | ||
| disabled: true, | ||
| sx={{ | ||
| ...position, | ||
| // reset dialog default styles | ||
| border: 'none', | ||
| padding: 0, | ||
| margin: 0, | ||
| '::backdrop': {background: 'transparent'}, | ||
| '& [data-selectpanel-primary-actions]': { | ||
| animation: footerAnimationEnabled ? 'selectpanel-gelatine 350ms linear' : 'none', | ||
| }, | ||
| '@keyframes selectpanel-gelatine': { | ||
| '0%': {transform: 'scale(1, 1)'}, | ||
| '25%': {transform: 'scale(0.9, 1.1)'}, | ||
| '50%': {transform: 'scale(1.1, 0.9)'}, | ||
| '75%': {transform: 'scale(0.95, 1.05)'}, | ||
| '100%': {transform: 'scale(1, 1)'}, | ||
| }, | ||
| }} | ||
| overlayProps={{ | ||
| role: 'dialog', | ||
| 'aria-labelledby': `${panelId}--title`, | ||
| 'aria-describedby': description ? `${panelId}--description` : undefined, | ||
| onClick={event => { | ||
| if (event.target === event.currentTarget) onClickOutside() | ||
| }} | ||
| > | ||
| <SelectPanelContext.Provider | ||
| @@ -171,15 +235,16 @@ const Panel: React.FC<SelectPanelProps> = ({ | ||
| > | ||
| <Box | ||
| as="form" | ||
| method="dialog" | ||
| onSubmit={onInternalSubmit} | ||
| sx={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| height: '100%', | ||
| }} | ||
| > | ||
| {/* render default header as fallback */} | ||
| {slots.header ?? <SelectPanelHeader />} | ||
| {slots.header ?? /* render default header as fallback */ <SelectPanelHeader />} | ||
| <Box | ||
| as="div" | ||
| ref={listContainerRef as React.RefObject<HTMLDivElement>} | ||
| @@ -209,7 +274,7 @@ const Panel: React.FC<SelectPanelProps> = ({ | ||
| {slots.footer} | ||
| </Box> | ||
| </SelectPanelContext.Provider> | ||
| </AnchoredOverlay> | ||
| </StyledOverlay> | ||
| </> | ||
| ) | ||
| } | ||
| @@ -279,6 +344,7 @@ const SelectPanelHeader: React.FC<React.PropsWithChildren> = ({children, ...prop | ||
| } | ||
| const SelectPanelSearchInput: React.FC<TextInputProps> = ({onChange: propsOnChange, ...props}) => { | ||
| // TODO: use forwardedRef | ||
| const inputRef = React.createRef<HTMLInputElement>() | ||
| const {setSearchQuery} = React.useContext(SelectPanelContext) | ||
| @@ -292,9 +358,6 @@ const SelectPanelSearchInput: React.FC<TextInputProps> = ({onChange: propsOnChan | ||
| return ( | ||
| <TextInput | ||
| // this autofocus doesn't seem to apply 🤔 | ||
| // probably because the focus zone overrides autoFocus | ||
| autoFocus | ||
| ref={inputRef} | ||
| block | ||
| leadingVisual={SearchIcon} | ||
| @@ -303,6 +366,7 @@ const SelectPanelSearchInput: React.FC<TextInputProps> = ({onChange: propsOnChan | ||
| <TextInput.Action | ||
| icon={XCircleFillIcon} | ||
| aria-label="Clear" | ||
| tooltipDirection="w" | ||
siddharthkp marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| sx={{color: 'fg.subtle', bg: 'none'}} | ||
| onClick={() => { | ||
| if (inputRef.current) inputRef.current.value = '' | ||
| @@ -349,7 +413,7 @@ const SelectPanelFooter = ({...props}) => { | ||
| <Box sx={{flexGrow: hidePrimaryActions ? 1 : 0}}>{props.children}</Box> | ||
| {hidePrimaryActions ? null : ( | ||
| <Box sx={{display: 'flex', gap: 2}}> | ||
| <Box data-selectpanel-primary-actions sx={{display: 'flex', gap: 2}}> | ||
| <Button size="small" type="button" onClick={() => onCancel()}> | ||
| Cancel | ||
| </Button> | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.