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

Adds open, onOpenChange, and anchorRef props to DropdownMenu #1372

Merged
merged 5 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changeset/great-hotels-remain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Extends DropdownMenu to allow anchorRef, open, and onOpenChange props.
53 changes: 34 additions & 19 deletions src/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import React, {useCallback, useMemo, useState} from 'react'
import React, {useCallback, useMemo} from 'react'
import {List, GroupedListProps, ListPropsBase, ItemInput} from '../ActionList/List'
import {DropdownButton, DropdownButtonProps} from './DropdownButton'
import {ItemProps} from '../ActionList/Item'
import {AnchoredOverlay} from '../AnchoredOverlay'
import {OverlayProps} from '../Overlay'
import {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay'
import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'

export interface DropdownMenuProps extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>, ListPropsBase {
/**
* A custom function component used to render the anchor element.
* Will receive the selected text as `children` prop when an item is activated.
* Uses a `DropdownButton` by default.
*/
renderAnchor?: <T extends React.HTMLAttributes<HTMLElement>>(props: T) => JSX.Element

interface DropdownMenuBaseProps extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>, ListPropsBase {
/**
* A placeholder value to display on the trigger button when no selection has been made.
*/
Expand All @@ -33,35 +29,53 @@ export interface DropdownMenuProps extends Partial<Omit<GroupedListProps, keyof
* Props to be spread on the internal `Overlay` component.
*/
overlayProps?: Partial<OverlayProps>

/**
* If defined, will control the open/closed state of the overlay. Must be used in conjuction with `onOpenChange`.
Copy link
Contributor

@colebemis colebemis Aug 10, 2021

Choose a reason for hiding this comment

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

Should we be explicit here about how the open state is managed if open is not defined? (i.e. it becomes an uncontrolled component)

*/
open?: boolean

/**
* If defined, will control the open/closed state of the overlay. Must be used in conjuction with `open`.
*/
onOpenChange?: (s: boolean) => void
}

export type DropdownMenuProps = DropdownMenuBaseProps & AnchoredOverlayWrapperAnchorProps

/**
* A `DropdownMenu` provides an anchor (button by default) that will open a floating menu of selectable items. The menu can be
* opened and navigated using keyboard or mouse. When an item is selected, the menu will close and the `onChange` callback will be called.
* If the default anchor button is used, the anchor contents will be updated with the selection.
*/
export function DropdownMenu({
renderAnchor = <T extends DropdownButtonProps>(props: T) => <DropdownButton {...props} />,
anchorRef: externalAnchorRef,
placeholder,
selectedItem,
onChange,
overlayProps,
items,
open,
onOpenChange,
...listProps
}: DropdownMenuProps): JSX.Element {
const [open, setOpen] = useState(false)
const onOpen = useCallback(() => setOpen(true), [])
const onClose = useCallback(() => setOpen(false), [])
const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, onOpenChange, false)
const onOpen = useCallback(() => setCombinedOpenState(true), [setCombinedOpenState])
const onClose = useCallback(() => setCombinedOpenState(false), [setCombinedOpenState])

const anchorRef = useProvidedRefOrCreate(externalAnchorRef)

const renderMenuAnchor = useCallback(
<T extends React.HTMLAttributes<HTMLElement>>(props: T) => {
return renderAnchor({
const renderMenuAnchor = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what happens if both renderAnchor and anchorRef are passed to DropdownMenu?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they're both passed, the renderAnchor function is called with anchorRef as a prop (which would mean putting the same ref in two DOM nodes, which seems wrong but I don't know how it'd break. It's almost certainly not an intended usage, but I'm aiming to mirror what I saw elsewhere.

I spoke with @dgreif about this yesterday, and it sounds like the prop types in AnchoredOverlay could have been narrowed (specifically in AnchoredOverlayPropsWithAnchor such that anchorRef did not exist), except it made destructuring props messier.

if (renderAnchor === null) {
return null
}
return <T extends React.HTMLAttributes<HTMLElement>>(props: T) =>
renderAnchor({
...props,
children: selectedItem?.text ?? placeholder
})
},
[placeholder, renderAnchor, selectedItem?.text]
)
}, [placeholder, renderAnchor, selectedItem?.text])

const itemsToRender = useMemo(() => {
return items.map(item => {
Expand All @@ -86,7 +100,8 @@ export function DropdownMenu({
return (
<AnchoredOverlay
renderAnchor={renderMenuAnchor}
open={open}
anchorRef={anchorRef}
open={combinedOpenState}
onOpen={onOpen}
onClose={onClose}
overlayProps={overlayProps}
Expand Down
Empty file added src/hooks/useAnchoredOverlay
Empty file.
30 changes: 30 additions & 0 deletions src/stories/DropdownMenu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react'
import {theme, ThemeProvider} from '..'
import {ItemInput} from '../ActionList/List'
import BaseStyles from '../BaseStyles'
import Box from '../Box'
import {DropdownMenu, DropdownButton} from '../DropdownMenu'
import TextInput from '../TextInput'

Expand Down Expand Up @@ -52,3 +53,32 @@ export function FavoriteColorStory(): JSX.Element {
)
}
FavoriteColorStory.storyName = 'Favorite Color'

export function ExternalAnchorStory(): JSX.Element {
const items = React.useMemo(() => [{text: '🔵 Cyan'}, {text: '🔴 Magenta'}, {text: '🟡 Yellow'}], [])
const [selectedItem, setSelectedItem] = React.useState<ItemInput | undefined>()
const anchorRef = React.useRef<HTMLDivElement>(null)
const [open, setOpen] = React.useState(false)

return (
<Box display="flex" flexDirection="column" alignItems="flex-start">
<Box display="flex" flexDirection="row">
<DropdownButton onClick={() => setOpen(true)}>Click me to open the dropdown</DropdownButton>
<Box ref={anchorRef} bg="papayawhip" p={4} ml={40} borderRadius={2} display="inline-block">
Anchored on me!
</Box>
</Box>
<DropdownMenu
renderAnchor={null}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is explicitly setting renderAnchor to null required in order to use anchorRef? If so, I can see that being a confusing API for consumers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I tried to mirror SelectPanel, where null and undefined have two distinct uses:

  • Passing undefined means that the component should provide a default renderAnchor method
  • null means that the component should not provide a default, and instead pass the null through to AnchoredOverlay (in the case where you wanted AnchoredOverlay to just use the anchorRef for anchoring).

It's a little wonky, but my inclination is to try to clean all of these up in a future PR. I'd like to hear your thoughts though!

anchorRef={anchorRef}
open={open}
onOpenChange={setOpen}
placeholder="🎨"
items={items}
selectedItem={selectedItem}
onChange={setSelectedItem}
/>
</Box>
)
}
ExternalAnchorStory.storyName = 'DropdownMenu with External Anchor'