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.

Sign up for GitHub

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: Make the Popover.Slot optional #53889

Merged
merged 5 commits into from
Aug 24, 2023
Merged

Components: Make the Popover.Slot optional #53889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db0d6b5
Components: Make the Popover.Slot optional
youknowriad Aug 23, 2023
8678bd9
Update changelog
youknowriad Aug 23, 2023
9a9ec37
Remove useless test
youknowriad Aug 23, 2023
d79b6b4
Keep rendering the block popover inline
youknowriad Aug 23, 2023
e17d499
Render inbetween inserter inline too
youknowriad Aug 23, 2023
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
3 changes: 0 additions & 3 deletions docs/how-to-guides/platform/custom-block-editor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,6 @@ function Editor( { settings } ) {
<Sidebar />
<BlockEditor settings={ settings } />
</div>
<Popover.Slot />
</DropZoneProvider>
</SlotFillProvider>
);
Expand All @@ -303,8 +302,6 @@ Let's examine these in more detail:
- `<Notices>` – Provides a "snack bar" Notice that will be rendered if any messages are dispatched to the `core/notices` store
- `<Header>` – Renders the static title "Standalone Block Editor" at the top of the editor UI
- `<BlockEditor>` – The custom block editor component
- `<Popover.Slot />` – Renders a slot into which `<Popover>`s can be rendered
using the Slot/Fill mechanic

### Keyboard navigation

Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ function MyEditorComponent() {
</ObserveTyping>
</WritingFlow>
</BlockTools>
<Popover.Slot />
</SlotFillProvider>
</BlockEditorProvider>
);
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/block-popover/inbetween.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ function BlockPopoverInbetween( {
focusOnMount={ false }
// Render in the old slot if needed for backward compatibility,
// otherwise render in place (not in the default popover slot).
__unstableSlotName={ __unstablePopoverSlot || null }
__unstableSlotName={ __unstablePopoverSlot }
inline={ ! __unstablePopoverSlot }
// Forces a remount of the popover when its position changes
// This makes sure the popover doesn't animate from its previous position.
key={ nextClientId + '--' + rootClientId }
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/block-popover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ function BlockPopover(
anchor={ popoverAnchor }
// Render in the old slot if needed for backward compatibility,
// otherwise render in place (not in the default popover slot).
__unstableSlotName={ __unstablePopoverSlot || null }
__unstableSlotName={ __unstablePopoverSlot }
inline={ ! __unstablePopoverSlot }
placement="top-start"
resize={ false }
flip={ false }
Expand Down
133 changes: 0 additions & 133 deletions packages/block-editor/src/components/url-popover/test/__snapshots__/index.js.snap
View file
Open in desktop

This file was deleted.

75 changes: 0 additions & 75 deletions packages/block-editor/src/components/url-popover/test/index.js
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Enhancements
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this a breaking change?

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 you were using Popover.Slot before, it would continue to work.

Some popovers might not render "inline" while they previously did, which could have subtle impacts but I'm not sure it's big enough to consider this a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely was, previously you could define styles taking advantage of the inline setup, but now CSS selectors need to be rewritten.


- Make the `Popover.Slot` optional and render popovers at the bottom of the document's body by default. ([#53889](https://github.com/WordPress/gutenberg/pull/53889)).

### Bug Fix

- `SandBox`: Fix the cleanup method in useEffect ([#53796](https://github.com/WordPress/gutenberg/pull/53796)).
Expand Down
11 changes: 3 additions & 8 deletions packages/components/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,9 @@ In non-WordPress projects, link to the `build-style/style.css` file directly, it

_If you're using [`Popover`](/packages/components/src/popover/README.md) or [`Tooltip`](/packages/components/src/tooltip/README.md) components outside of the editor, make sure they are rendered within a `SlotFillProvider` and with a `Popover.Slot` somewhere up the element tree._

By default, the `Popover` component will render inline i.e. within its
parent to which it should anchor. Depending upon the context in which the
`Popover` is being consumed, this might lead to incorrect positioning. For
example, when being nested within another popover.

This issue can be solved by rendering popovers to a specific location in the DOM via the
`Popover.Slot`. For this to work, you will need your use of the `Popover`
component and its `Slot` to be wrapped in a [`SlotFill`](/packages/components/src/slot-fill/README.md) provider.
By default, the `Popover` component will render within an extra element appended to the body of the document.

If you want to precisely contol where the popovers render, you will need to use the `Popover.Slot` component.

A `Popover` is also used as the underlying mechanism to display `Tooltip` components.
So the same considerations should be applied to them.
Expand Down
8 changes: 2 additions & 6 deletions packages/components/src/border-box-control/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import { useState } from '@wordpress/element';
* Internal dependencies
*/
import Button from '../../button';
import Popover from '../../popover';
import { BorderBoxControl } from '../';
import { Provider as SlotFillProvider } from '../../slot-fill';

const meta: Meta< typeof BorderBoxControl > = {
title: 'Components (Experimental)/BorderBoxControl',
Expand Down Expand Up @@ -53,7 +51,7 @@ const Template: StoryFn< typeof BorderBoxControl > = ( props ) => {
};

return (
<SlotFillProvider>
<>
<BorderBoxControl
{ ...otherProps }
onChange={ onChangeMerged }
Expand All @@ -78,9 +76,7 @@ const Template: StoryFn< typeof BorderBoxControl > = ( props ) => {
>
Reset
</Button>
{ /* @ts-expect-error Ignore until Popover.Slot is converted to TS */ }
<Popover.Slot />
</SlotFillProvider>
</>
);
};
export const Default = Template.bind( {} );
Expand Down
16 changes: 5 additions & 11 deletions packages/components/src/border-control/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import { useState } from '@wordpress/element';
* Internal dependencies
*/
import { BorderControl } from '..';
import { Provider as SlotFillProvider } from '../../slot-fill';
import Popover from '../../popover';
import type { Border } from '../types';

const meta: Meta< typeof BorderControl > = {
Expand Down Expand Up @@ -83,15 +81,11 @@ const Template: StoryFn< typeof BorderControl > = ( {
};

return (
<SlotFillProvider>
<BorderControl
onChange={ onChangeMerged }
value={ border }
{ ...props }
/>
{ /* @ts-expect-error Ignore until Popover.Slot is converted to TS */ }
<Popover.Slot />
</SlotFillProvider>
<BorderControl
onChange={ onChangeMerged }
value={ border }
{ ...props }
/>
);
};

Expand Down
22 changes: 8 additions & 14 deletions packages/components/src/color-palette/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import { useState } from '@wordpress/element';
* Internal dependencies
*/
import ColorPalette from '..';
import Popover from '../../popover';
import { Provider as SlotFillProvider } from '../../slot-fill';

const meta: Meta< typeof ColorPalette > = {
title: 'Components/ColorPalette',
Expand All @@ -35,18 +33,14 @@ const Template: StoryFn< typeof ColorPalette > = ( { onChange, ...args } ) => {
const [ color, setColor ] = useState< string | undefined >();

return (
<SlotFillProvider>
<ColorPalette
{ ...args }
value={ color }
onChange={ ( newColor ) => {
setColor( newColor );
onChange?.( newColor );
} }
/>
{ /* @ts-expect-error The 'Slot' component hasn't been typed yet. */ }
<Popover.Slot />
</SlotFillProvider>
<ColorPalette
{ ...args }
value={ color }
onChange={ ( newColor ) => {
setColor( newColor );
onChange?.( newColor );
} }
/>
);
};

Expand Down
Loading