-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Components: promote g2 Popover
as Flyout
#32197
Merged
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
e2efe53
Move `components/src/ui/popover` to `components/src/accessible-popover`
ciampo 650deeb
Add to Docs manifest
ciampo 777f822
Add to tsconfig.json
ciampo 8363483
Rename context-related variables from Popover* to AccessiblePopover*
ciampo 8e0d236
Rename `PopoverContext` type to `Context`
ciampo 59c21e4
Rename `PopoverContent` to `AccessiblePopoverContent`
ciampo 935bc5e
Move `component.js` to component-specific folder
ciampo ed58977
Move `AccessibilePopover` logic to separate hook
ciampo 083fe44
Move `content.js` to component-specific folder
ciampo 5c15310
Move `AccessibilePopoverContent` logic to separate hook
ciampo 1e68e3c
Rename `AccessiblePopover` to `Flyout`
ciampo 5d3428c
Rename zIndex const from `Popover` to `Flyout`
ciampo 2d92a1e
Do not export FlyoutContent component
ciampo 294279d
Fix imports in example
ciampo 337249a
Rename `popover` to `flyoutState`
ciampo cd5d33b
Refactor to `styled` approach
ciampo 91857d9
Wrap `ReakitPopover` in `FlyoutContentView` and set `maxWidth` in inl…
ciampo 8d31c25
auto-format
ciampo db1cbae
Add Props documentation to Flyout
ciampo 00cc295
Mark `Flyout` as non-polymorphic
ciampo 1b5f2a5
Add documentation to `FlyoutContent`s README
ciampo a7389ba
Update snapshots
ciampo 23b945e
Remove FlyoutContext exports
ciampo edfb229
Do not export FlyoutContent`s documentation
ciampo 2b5d016
Delete FlyoutContent`s README
ciampo 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
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,10 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { createContext, useContext } from '@wordpress/element'; | ||
|
||
/** | ||
* @type {import('react').Context<import('./types').Context>} | ||
*/ | ||
export const FlyoutContext = createContext( {} ); | ||
export const useFlyoutContext = () => useContext( FlyoutContext ); |
53 changes: 53 additions & 0 deletions
53
packages/components/src/flyout/flyout-content/component.js
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,53 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { useFlyoutContext } from '../context'; | ||
import { FlyoutContentView, CardView } from '../styles'; | ||
import { contextConnect, useContextSystem } from '../../ui/context'; | ||
|
||
/** | ||
* | ||
* @param {import('../../ui/context').PolymorphicComponentProps<import('../types').ContentProps, 'div', false>} props | ||
* @param {import('react').Ref<any>} forwardedRef | ||
*/ | ||
function FlyoutContent( props, forwardedRef ) { | ||
const { | ||
children, | ||
elevation, | ||
maxWidth, | ||
style = {}, | ||
...otherProps | ||
} = useContextSystem( props, 'FlyoutContent' ); | ||
|
||
const { label, flyoutState } = useFlyoutContext(); | ||
|
||
if ( ! flyoutState ) { | ||
throw new Error( | ||
'`FlyoutContent` must only be used inside a `Flyout`.' | ||
); | ||
} | ||
|
||
const showContent = flyoutState.visible || flyoutState.animating; | ||
|
||
return ( | ||
<FlyoutContentView | ||
aria-label={ label } | ||
// maxWidth is applied via inline styles in order to avoid the `React does | ||
// not recognize the maxWidth prop on a DOM element` error that comes from | ||
// passing `maxWidth` as a prop to `FlyoutContentView` | ||
style={ { maxWidth, ...style } } | ||
{ ...otherProps } | ||
{ ...flyoutState } | ||
> | ||
{ showContent && ( | ||
<CardView elevation={ elevation } ref={ forwardedRef }> | ||
{ children } | ||
</CardView> | ||
) } | ||
</FlyoutContentView> | ||
); | ||
} | ||
|
||
const ConnectedFlyoutContent = contextConnect( FlyoutContent, 'FlyoutContent' ); | ||
|
||
export default ConnectedFlyoutContent; |
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 @@ | ||
export { default } from './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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
# Flyout | ||
|
||
<div class="callout callout-alert"> | ||
This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. | ||
</div> | ||
|
||
`Flyout` is a component to render a floating content modal. It is similar in purpose to a tooltip, but renders content of any sort, not only simple text. | ||
|
||
## Usage | ||
|
||
```jsx | ||
import { Button, __experimentalFlyout as Flyout, __experimentalText as } from '@wordpress/components'; | ||
|
||
function Example() { | ||
return ( | ||
<Flyout trigger={ <Button>Show/Hide content</Button> }> | ||
<Text>Code is Poetry</Text> | ||
</Flyout> | ||
); | ||
} | ||
``` | ||
|
||
## Props | ||
|
||
### `state`: `PopoverStateReturn` | ||
|
||
- Required: No | ||
|
||
### `label`: `string` | ||
|
||
- Required: No | ||
|
||
### `animated`: `boolean` | ||
|
||
Determines if `Flyout` has animations. | ||
|
||
- Required: No | ||
- Default: `true` | ||
|
||
### `animationDuration`: `boolean` | ||
|
||
The duration of `Flyout` animations. | ||
|
||
- Required: No | ||
- Default: `160` | ||
|
||
### `baseId`: `string` | ||
|
||
ID that will serve as a base for all the items IDs. See https://reakit.io/docs/popover/#usepopoverstate | ||
|
||
- Required: No | ||
- Default: `160` | ||
|
||
### `elevation`: `number` | ||
|
||
Size of the elevation shadow. For more information, check out [`Card`](/packages/components/src/card/card/README.md#props). | ||
|
||
- Required: No | ||
- Default: `5` | ||
|
||
### `maxWidth`: `CSSProperties[ 'maxWidth' ]` | ||
|
||
Max-width for the `Flyout` element. | ||
|
||
- Required: No | ||
- Default: `360` | ||
|
||
### `onVisibleChange`: `( ...args: any ) => void` | ||
|
||
Callback for when the `visible` state changes. | ||
|
||
- Required: No | ||
|
||
### `trigger`: `FunctionComponentElement< any >` | ||
|
||
Element that triggers the `visible` state of `Flyout` when clicked. | ||
|
||
```jsx | ||
<Flyout trigger={<Button>Greet</Button>}> | ||
<Text>Hi! I'm Olaf!</Text> | ||
</Flyout> | ||
``` | ||
|
||
- Required: Yes | ||
|
||
### `visible`: `boolean` | ||
|
||
Whether `Flyout` is visible. See [the `Reakit` docs](https://reakit.io/docs/popover/#usepopoverstate) for more information. | ||
|
||
- Required: No | ||
- Default: `false` | ||
|
||
### `placement`: `PopperPlacement` | ||
|
||
Position of the popover element. See [the `popper` docs](https://popper.js.org/docs/v1/#popperplacements--codeenumcode) for more information. | ||
|
||
- Required: No | ||
- Default: `auto` |
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,111 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
// eslint-disable-next-line no-restricted-imports | ||
import { PopoverDisclosure, Portal } from 'reakit'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useCallback, useMemo, cloneElement } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { contextConnect } from '../../ui/context'; | ||
import { FlyoutContext } from '../context'; | ||
import { useFlyoutResizeUpdater } from '../utils'; | ||
import FlyoutContent from '../flyout-content'; | ||
import { useUpdateEffect } from '../../utils/hooks'; | ||
import { useFlyout } from './hook'; | ||
|
||
/** | ||
* | ||
* @param {import('../../ui/context').PolymorphicComponentProps<import('../types').Props, 'div', false>} props | ||
* @param {import('react').Ref<any>} forwardedRef | ||
*/ | ||
function Flyout( props, forwardedRef ) { | ||
const { | ||
children, | ||
elevation, | ||
label, | ||
maxWidth, | ||
onVisibleChange, | ||
trigger, | ||
flyoutState, | ||
...otherProps | ||
} = useFlyout( props ); | ||
|
||
const resizeListener = useFlyoutResizeUpdater( { | ||
onResize: flyoutState.unstable_update, | ||
} ); | ||
|
||
const uniqueId = `flyout-${ flyoutState.baseId }`; | ||
const labelId = label || uniqueId; | ||
|
||
const contextProps = useMemo( | ||
() => ( { | ||
label: labelId, | ||
flyoutState, | ||
} ), | ||
[ labelId, flyoutState ] | ||
); | ||
|
||
const triggerContent = useCallback( | ||
( triggerProps ) => { | ||
return cloneElement( trigger, triggerProps ); | ||
}, | ||
[ trigger ] | ||
); | ||
|
||
useUpdateEffect( () => { | ||
onVisibleChange?.( flyoutState.visible ); | ||
}, [ flyoutState.visible ] ); | ||
|
||
return ( | ||
<FlyoutContext.Provider value={ contextProps }> | ||
{ trigger && ( | ||
<PopoverDisclosure | ||
{ ...flyoutState } | ||
ref={ trigger.ref } | ||
{ ...trigger.props } | ||
> | ||
{ triggerContent } | ||
</PopoverDisclosure> | ||
) } | ||
<Portal> | ||
<FlyoutContent | ||
ref={ forwardedRef } | ||
{ ...otherProps } | ||
elevation={ elevation } | ||
maxWidth={ maxWidth } | ||
> | ||
{ resizeListener } | ||
{ children } | ||
</FlyoutContent> | ||
</Portal> | ||
</FlyoutContext.Provider> | ||
); | ||
} | ||
|
||
/** | ||
* `Flyout` is a component to render a floating content modal. | ||
* It is similar in purpose to a tooltip, but renders content of any sort, | ||
* not only simple text. | ||
* | ||
* @example | ||
* ```jsx | ||
* import { Button, __experimentalFlyout as Flyout, __experimentalText as } from '@wordpress/components'; | ||
* | ||
* function Example() { | ||
* return ( | ||
* <Flyout trigger={ <Button>Show/Hide content</Button> }> | ||
* <Text>Code is Poetry</Text> | ||
* </Flyout> | ||
* ); | ||
* } | ||
* ``` | ||
*/ | ||
const ConnectedFlyout = contextConnect( Flyout, 'Flyout' ); | ||
|
||
export default ConnectedFlyout; |
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,45 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
// eslint-disable-next-line no-restricted-imports | ||
import { usePopoverState } from 'reakit'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { useContextSystem } from '../../ui/context'; | ||
|
||
/** | ||
* @param {import('../../ui/context').PolymorphicComponentProps<import('../types').Props, 'div', false>} props | ||
*/ | ||
export function useFlyout( props ) { | ||
const { | ||
animated = true, | ||
animationDuration = 160, | ||
baseId, | ||
elevation = 5, | ||
id, | ||
maxWidth = 360, | ||
placement, | ||
state, | ||
visible, | ||
...otherProps | ||
} = useContextSystem( props, 'Flyout' ); | ||
|
||
const _flyoutState = usePopoverState( { | ||
animated: animated ? animationDuration : undefined, | ||
baseId: baseId || id, | ||
placement, | ||
visible, | ||
...otherProps, | ||
} ); | ||
|
||
const flyoutState = state || _flyoutState; | ||
|
||
return { | ||
...otherProps, | ||
elevation, | ||
maxWidth, | ||
flyoutState, | ||
}; | ||
} |
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,2 @@ | ||
export { default } from './component'; | ||
export { useFlyout } from './hook'; |
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 @@ | ||
export { default as Flyout } from './flyout'; |
14 changes: 7 additions & 7 deletions
14
...omponents/src/ui/popover/stories/index.js → ...es/components/src/flyout/stories/index.js
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 |
---|---|---|
@@ -1,24 +1,24 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { CardBody, CardHeader } from '../../../card'; | ||
import Button from '../../../button'; | ||
import { Popover } from '..'; | ||
import { CardBody, CardHeader } from '../../card'; | ||
import Button from '../../button'; | ||
import { Flyout } from '..'; | ||
|
||
export default { | ||
component: Popover, | ||
title: 'G2 Components (Experimental)/Popover', | ||
component: Flyout, | ||
title: 'Components (Experimental)/Flyout', | ||
}; | ||
|
||
export const _default = () => { | ||
return ( | ||
<Popover | ||
<Flyout | ||
trigger={ <Button>Click</Button> } | ||
visible | ||
placement="bottom-start" | ||
> | ||
<CardHeader>Go</CardHeader> | ||
<CardBody>Stuff</CardBody> | ||
</Popover> | ||
</Flyout> | ||
); | ||
}; |
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.
The refactoring done in 3598b73, (where we wrapped
ReakitPopover
directly into thestyled
function), caused a typescript error around theas
prop on< FlyoutContentView />
.For this reason,
FlyoutContent
(andFlyout
) have been marked as non-polymorphicThere 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.
Out of curiosity, what's the context behind refactoring the
css
calls tostyled.div
and then tostyled( ReakitPopover )
? I remember we've had some conversations about that, but I can't remember exactly the reasoning behind it. cc @sarayourfriendThere 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.
Basically
@emotion/css
does not hook into the Emotion cache so it's not possible for us to use it'scss
function because we need to hook into the cache to support iframes.@emotion/styled
does hook into the CacheProvider so it is able to support iframes.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.
Ah yeah! That makes sense.