-
Notifications
You must be signed in to change notification settings - Fork 16
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(ui): Popover #1696
Merged
Merged
feat(ui): Popover #1696
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
dbb9e3f
feat(ui): #prax-156: implement popover component
VanishMax e42ff84
feat(ui): #prax-156: add tests for Popover
VanishMax d4ec7d5
feat(ui): #prax-156: add appear animation
VanishMax e33e6f5
chore: format
VanishMax a3106d8
chore: changeset
VanishMax f8fa2c6
fix(ui): #prax-156: update popover based on the comments
VanishMax File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 @@ | ||
--- | ||
'@repo/ui': minor | ||
--- | ||
|
||
Add Popover UI component |
This file contains 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 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 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,67 @@ | ||
import type { Meta, StoryObj } from '@storybook/react'; | ||
|
||
import { Popover } from '.'; | ||
import { Button } from '../Button'; | ||
import { ComponentType, useState } from 'react'; | ||
import { Text } from '../Text'; | ||
import styled from 'styled-components'; | ||
import { Shield } from 'lucide-react'; | ||
import { Density } from '../Density'; | ||
|
||
const Wrapper = styled.div` | ||
display: flex; | ||
flex-direction: column; | ||
gap: ${props => props.theme.spacing(4)}; | ||
color: ${props => props.theme.color.text.primary}; | ||
`; | ||
|
||
const meta: Meta<typeof Popover> = { | ||
component: Popover, | ||
tags: ['autodocs', '!dev'], | ||
argTypes: { | ||
isOpen: { control: false }, | ||
onClose: { control: false }, | ||
}, | ||
subcomponents: { | ||
// Re: type coercion, see | ||
// https://github.com/storybookjs/storybook/issues/23170#issuecomment-2241802787 | ||
'Popover.Content': Popover.Content as ComponentType<unknown>, | ||
'Popover.Trigger': Popover.Trigger as ComponentType<unknown>, | ||
}, | ||
}; | ||
export default meta; | ||
|
||
type Story = StoryObj<typeof Popover>; | ||
|
||
export const Basic: Story = { | ||
render: function Render() { | ||
const [isOpen, setIsOpen] = useState(false); | ||
|
||
return ( | ||
<Popover isOpen={isOpen} onClose={() => setIsOpen(false)}> | ||
<Popover.Trigger asChild> | ||
<Button onClick={() => setIsOpen(true)}>Open popover</Button> | ||
</Popover.Trigger> | ||
|
||
<Popover.Content> | ||
<Wrapper> | ||
<Text body as='h3'> | ||
This is a heading | ||
</Text> | ||
<Text small> | ||
This is description information. Lorem ipsum dolor sit amet, consectetur adipiscing | ||
elit. Ut et massa mi. | ||
</Text> | ||
<div> | ||
<Density compact> | ||
<Button icon={Shield} onClick={() => setIsOpen(false)}> | ||
Action | ||
</Button> | ||
</Density> | ||
</div> | ||
</Wrapper> | ||
</Popover.Content> | ||
</Popover> | ||
); | ||
}, | ||
}; |
This file contains 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,32 @@ | ||
import { fireEvent, render } from '@testing-library/react'; | ||
import { describe, expect, it } from 'vitest'; | ||
import { Popover } from '.'; | ||
import { PenumbraUIProvider } from '../PenumbraUIProvider'; | ||
|
||
describe('<Popover />', () => { | ||
it('opens when trigger is clicked', () => { | ||
const { getByText, queryByText } = render( | ||
<Popover> | ||
<Popover.Trigger>Trigger</Popover.Trigger> | ||
<Popover.Content>Content</Popover.Content> | ||
</Popover>, | ||
{ wrapper: PenumbraUIProvider }, | ||
); | ||
|
||
expect(queryByText('Content')).toBeFalsy(); | ||
fireEvent.click(getByText('Trigger')); | ||
expect(queryByText('Content')).toBeTruthy(); | ||
}); | ||
|
||
it('opens initially if `isOpen` is passed', () => { | ||
const { queryByText } = render( | ||
<Popover isOpen> | ||
<Popover.Trigger>Trigger</Popover.Trigger> | ||
<Popover.Content>Content</Popover.Content> | ||
</Popover>, | ||
{ wrapper: PenumbraUIProvider }, | ||
); | ||
|
||
expect(queryByText('Content')).toBeTruthy(); | ||
}); | ||
}); |
This file contains 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,169 @@ | ||
import { ReactNode } from 'react'; | ||
import * as RadixPopover from '@radix-ui/react-popover'; | ||
import type { PopoverContentProps as RadixPopoverContentProps } from '@radix-ui/react-popover'; | ||
import styled, { keyframes, useTheme } from 'styled-components'; | ||
|
||
const scaleIn = keyframes` | ||
from { | ||
opacity: 0; | ||
transform: scale(0); | ||
} | ||
to { | ||
opacity: 1; | ||
transform: scale(1); | ||
} | ||
`; | ||
|
||
const RadixContent = styled.div` | ||
display: flex; | ||
flex-direction: column; | ||
gap: ${props => props.theme.spacing(4)}; | ||
|
||
width: 240px; | ||
max-width: 320px; | ||
padding: ${props => props.theme.spacing(3)} ${props => props.theme.spacing(2)}; | ||
|
||
background: ${props => props.theme.color.other.dialogBackground}; | ||
border: 1px solid ${props => props.theme.color.other.tonalStroke}; | ||
border-radius: ${props => props.theme.borderRadius.sm}; | ||
backdrop-filter: blur(${props => props.theme.blur.lg}); | ||
|
||
transform-origin: var(--radix-tooltip-content-transform-origin); | ||
animation: ${scaleIn} 0.15s ease-out; | ||
`; | ||
|
||
interface ControlledPopoverProps { | ||
/** | ||
* Whether the popover is currently open. If left `undefined`, this will be | ||
* treated as an uncontrolled popover — that is, it will open and close based | ||
* on user interactions rather than on state variables. | ||
*/ | ||
isOpen: boolean; | ||
/** | ||
* Callback for when the user closes the popover. Should update the state | ||
* variable being passed in via `isOpen`. If left `undefined`, users will not | ||
* be able to close it -- that is, it will only be able to be closed programmatically | ||
*/ | ||
onClose?: VoidFunction; | ||
} | ||
|
||
interface UncontrolledPopoverProps { | ||
isOpen?: undefined; | ||
onClose?: undefined; | ||
} | ||
|
||
export type PopoverProps = { | ||
children?: ReactNode; | ||
} & (ControlledPopoverProps | UncontrolledPopoverProps); | ||
|
||
/** | ||
* A popover box that appears next to the trigger element. | ||
* | ||
* To render a popover, compose it using a few components: `<Popover />`, | ||
* `<Popover.Trigger />`, and `<Popover.Content />`. The latter two must be | ||
* descendents of `<Popover />` in the component tree, and siblings to each | ||
* other. (`<Popover.Trigger />` is optional, though — more on that in a moment.) | ||
* | ||
* ```tsx | ||
* <Popover> | ||
* <Popover.Trigger asChild> | ||
* <Button>Open the popover</Button> | ||
* </Popover.Trigger> | ||
* | ||
* <Popover.Content title="Popover title">Popover content here</Popover.Content> | ||
* </Popover> | ||
* ``` | ||
* | ||
* Depending on your use case, you may want to use `<Popover />` either as a | ||
* controlled component, or as an uncontrolled component. | ||
* | ||
* ## Usage as a controlled component | ||
* | ||
* Use `<Popover />` as a controlled component when you want to control its | ||
* open/closed state yourself (e.g., via a state management solution like | ||
* Zustand or Redux). You can accomplish this by passing `isOpen` and `onClose` | ||
* props to the `<Popover />` component, and omitting `<Popover.Trigger />`: | ||
* | ||
* ```tsx | ||
* <Button onClick={() => setIsOpen(true)}>Open popover</Button> | ||
* | ||
* <Popover isOpen={isOpen} onClose={() => setIsOpen(false)}> | ||
* <Popover.Content title="Popover title">Popover content here</Popover.Content> | ||
* </Popover> | ||
* ``` | ||
* | ||
* Note that, in the example above, the `<Button />` lives outside of the | ||
* `<Popover />`, and there is no `<Popover.Trigger />` component rendered inside | ||
* the `<Popover />`. | ||
* | ||
* ## Usage as an uncontrolled component | ||
* | ||
* If you want to render `<Popover />` as an uncontrolled component, don't pass | ||
* `isOpen` or `onClose` to `<Popover />`, and make sure to include a | ||
* `<Popover.Trigger />` component inside the `<Popover />`: | ||
|
||
* ```tsx | ||
* <Popover> | ||
* <Popover.Trigger asChild> | ||
* <Button>Open the popover</Button> | ||
* </Popover.Trigger> | ||
* | ||
* <Popover.Content title="Popover title">Popover content here</Popover.Content> | ||
* </Popover> | ||
* ``` | ||
*/ | ||
export const Popover = ({ children, onClose, isOpen }: PopoverProps) => { | ||
return ( | ||
<RadixPopover.Root open={isOpen} onOpenChange={value => onClose && !value && onClose()}> | ||
{children} | ||
</RadixPopover.Root> | ||
); | ||
}; | ||
|
||
export interface PopoverTriggerProps { | ||
children: ReactNode; | ||
/** | ||
* Change the default rendered element for the one passed as a child, merging | ||
* their props and behavior. | ||
* | ||
* Uses Radix UI's `asChild` prop under the hood. | ||
* | ||
* @see https://www.radix-ui.com/primitives/docs/guides/composition | ||
*/ | ||
asChild?: boolean; | ||
} | ||
|
||
const Trigger = ({ children, asChild }: PopoverTriggerProps) => ( | ||
<RadixPopover.Trigger asChild={asChild}>{children}</RadixPopover.Trigger> | ||
); | ||
Popover.Trigger = Trigger; | ||
|
||
export interface PopoverContentProps { | ||
children?: ReactNode; | ||
side?: RadixPopoverContentProps['side']; | ||
align?: RadixPopoverContentProps['align']; | ||
} | ||
|
||
/** | ||
* Popover content. Must be a child of `<Popover />`. | ||
* | ||
* Control the position of the Popover relative to the trigger element by passing | ||
* `side` and `align` props. | ||
*/ | ||
const Content = ({ children, side, align }: PopoverContentProps) => { | ||
const theme = useTheme(); | ||
|
||
return ( | ||
<RadixPopover.Portal> | ||
<RadixPopover.Content | ||
sideOffset={theme.spacing(1, 'number')} | ||
side={side} | ||
align={align} | ||
asChild | ||
> | ||
<RadixContent>{children}</RadixContent> | ||
</RadixPopover.Content> | ||
</RadixPopover.Portal> | ||
); | ||
}; | ||
Popover.Content = Content; |
Oops, something went wrong.
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.
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.
thought: This is prob fine, but for what it's worth, I'm a little wary of giving consumers too much control.
I feel like it's generally better to reduce the number of props we expose, and try to restrict usage to the designs Sam gives us. That said, I can see how users might need the side/alignment to be different for layout reasons, so this is fine.
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.
In this case, I couldn't figure out how to control the position on our side. Sam wrote in Figma that it should open "on the left side if above the trigger" and "on the right side if below the trigger". Radix doesn't allow to set it dynamically - we can choose only one option. So that's why I decided to give this level of control