feat(Popover): New Popover component
#1595
Conversation
nikitaorliak-cengage
added
the
draft
🦋 Changeset detectedLatest commit: cfee18f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| import { useIsInverse } from '../../inverse'; | ||
| import { ButtonGroupContext } from '../ButtonGroup'; | ||
|
|
||
| export enum PopoverPositioning { |
There was a problem hiding this comment.
To match all our other similar props, let's rename this PopoverPosition
| left = 'left', | ||
| right = 'right', |
There was a problem hiding this comment.
From the requirements, it's unclear to me if we want to support left and right as well. @orion-cengage ?
| right = 'right', | ||
| } | ||
|
|
||
| export interface PopoverProps extends React.HTMLAttributes<HTMLDivElement> { |
There was a problem hiding this comment.
We should add descriptions to all of these props
| focusable?: boolean; | ||
| isInverse?: boolean; | ||
| isDisabled?: boolean; | ||
| matchedWidth?: boolean; |
There was a problem hiding this comment.
I wonder if we can somehow combine width and matchedWidth so we only use one prop to manage the width
There was a problem hiding this comment.
Mantine does width="target" for matching the width to the target element, or it takes a number for it to be a fixed width. Maybe we can do the same?
| isInverse?: boolean; | ||
| isDisabled?: boolean; | ||
| matchedWidth?: boolean; | ||
| withoutPointer?: boolean; |
There was a problem hiding this comment.
Maybe we make this positive and rename it hasPointer or hasArrow
| /* display: grid; */ | ||
| /* grid-template-rows: 1fr auto 1fr; */ | ||
| /* flex-wrap: wrap; */ | ||
| /* justify-content: center; */ | ||
| /* align-items: center; */ |
| z-index: 2; | ||
| `; | ||
|
|
||
| const PopoverChildrenContent = styled.div``; |
There was a problem hiding this comment.
If there are no styles, we can just use a div without the name
| > | ||
| <StyledAnnounce maxHeight={context.maxHeight}> | ||
| {!content.length ? ( | ||
| 'Content must be passed' |
There was a problem hiding this comment.
Not sure we should announce an "error" for the developers. I think we can assume there is content?
There was a problem hiding this comment.
Are the header and footer practically the same?
There was a problem hiding this comment.
We should add more examples to match all the ones provided in the designs
…e props, change matchedWidth to width=target, add more examples, fix errors
Issue: #927
What I did
Created a new
Popovercomponent.Screenshots:
Checklist
How to test