Skip to content

Commit

Permalink
Components: Allow style prop on Popover (#64489)
Browse files Browse the repository at this point in the history
* Allow style prop on popover

* Ensure styles are not overwritten by popoverProps

* Add tests and fix overriding of built-in popover styles

* Update comment

* Remove blank line

* Un-should tests

* Update test to use `not.toHaveStyle

* Add changelog entry

* Remove `style` from types file

----

Co-authored-by: talldan <talldanwp@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
  • Loading branch information
3 people authored Aug 15, 2024
1 parent d669eb6 commit c8eaf5c
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 3 deletions.
2 changes: 2 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
- `TimePicker`: add `hideLabelFromVision` prop ([#64267](https://github.com/WordPress/gutenberg/pull/64267)).
- `FocalPointPicker`: Default to new 40px size ([#64456](https://github.com/WordPress/gutenberg/pull/64456)).
- `DropdownMenuV2`: adopt elevation scale ([#64432](https://github.com/WordPress/gutenberg/pull/64432)).
- `Popover`: allow `style` prop usage ([#64489](https://github.com/WordPress/gutenberg/pull/64489)).


### Bug Fixes

Expand Down
12 changes: 9 additions & 3 deletions packages/components/src/popover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ const UnforwardedPopover = (
WordPressComponentProps< PopoverProps, 'div', false >,
// To avoid overlaps between the standard HTML attributes and the props
// expected by `framer-motion`, omit all framer motion props from popover
// props (except for `animate` and `children`, which are re-defined in `PopoverProps`).
keyof Omit< MotionProps, 'animate' | 'children' >
// props (except for `animate` and `children` which are re-defined in
// `PopoverProps`, and `style` which is merged safely).
keyof Omit< MotionProps, 'animate' | 'children' | 'style' >
>,
forwardedRef: ForwardedRef< any >
) => {
Expand All @@ -139,6 +140,7 @@ const UnforwardedPopover = (
shift = false,
inline = false,
variant,
style: contentStyle,

// Deprecated props
__unstableForcePosition,
Expand Down Expand Up @@ -370,6 +372,7 @@ const UnforwardedPopover = (
const animationProps: HTMLMotionProps< 'div' > = shouldAnimate
? {
style: {
...contentStyle,
...motionInlineStyles,
...style,
},
Expand All @@ -378,7 +381,10 @@ const UnforwardedPopover = (
}
: {
animate: false,
style,
style: {
...contentStyle,
...style,
},
};

// When Floating UI has finished positioning and Framer Motion has finished animating
Expand Down
34 changes: 34 additions & 0 deletions packages/components/src/popover/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,40 @@ describe( 'Popover', () => {
} );
} );

describe( 'style', () => {
it( 'outputs inline styles added through the style prop in addition to built-in popover positioning styles', async () => {
render(
<Popover
style={ { zIndex: 0 } }
data-testid="popover-element"
>
Hello
</Popover>
);
const popover = screen.getByTestId( 'popover-element' );

await waitFor( () => expect( popover ).toBeVisible() );
expect( popover ).toHaveStyle(
'position: absolute; top: 0px; left: 0px; z-index: 0;'
);
} );

it( 'is not possible to override built-in popover positioning styles via the style prop', async () => {
render(
<Popover
style={ { position: 'static' } }
data-testid="popover-element"
>
Hello
</Popover>
);
const popover = screen.getByTestId( 'popover-element' );

await waitFor( () => expect( popover ).toBeVisible() );
expect( popover ).not.toHaveStyle( 'position: static;' );
} );
} );

describe( 'focus behavior', () => {
it( 'should focus the popover container when opened', async () => {
render(
Expand Down

0 comments on commit c8eaf5c

Please sign in to comment.