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

Popover: refine position-to-placement conversion logic, add tests #44377

Merged
merged 5 commits into from
Sep 23, 2022
Merged

Popover: refine position-to-placement conversion logic, add tests #44377

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
fb8074c
Allow single-value `position` prop with yAxis values
ciampo Sep 22, 2022
a97fa9f
Add more comprehensive list of position-to-placement tests
ciampo Sep 22, 2022
88dc26a
Replace custom logic in positionToPlacement with a map of position-to…
ciampo Sep 22, 2022
9678180
CHANGELOG
ciampo Sep 22, 2022
3113b35
Fix typo
ciampo Sep 23, 2022
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
13 changes: 10 additions & 3 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,16 @@

## Unreleased

### Bug Fix

- `Popover`: fix limitShift logic by adding iframe offset correctly [#42950](https://github.com/WordPress/gutenberg/pull/42950)).
- `Popover`: refine position-to-placement conversion logic, add tests ([#44377](https://github.com/WordPress/gutenberg/pull/44377)).

### Internal

- `Mobile` updated to ignore `react/exhaustive-deps` eslint rule ([#44207](https://github.com/WordPress/gutenberg/pull/44207)).
- `Popover`: refactor unit tests to TypeScript and modern RTL assertions ([#44373](https://github.com/WordPress/gutenberg/pull/44373)).

Comment on lines +5 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also moved a few changelog entries from recent PRs to the Unreleased section, since they had been wrongly added under the 21.1.0 release

## 21.1.0 (2022-09-21)

### Deprecations
Expand All @@ -12,16 +22,13 @@

- `Button`: Remove unexpected `has-text` class when empty children are passed ([#44198](https://github.com/WordPress/gutenberg/pull/44198)).
- The `LinkedButton` to unlink sides in `BoxControl`, `BorderBoxControl` and `BorderRadiusControl` have changed from a rectangular primary button to an icon-only button, with a sentence case tooltip, and default-size icon for better legibility. The `Button` component has been fixed so when `isSmall` and `icon` props are set, and no text is present, the button shape is square rather than rectangular.
- `Popover`: fix limitShift logic by adding iframe offset correctly [#42950](https://github.com/WordPress/gutenberg/pull/42950)).

### Internal

- `NavigationMenu` updated to ignore `react/exhaustive-deps` eslint rule ([#44090](https://github.com/WordPress/gutenberg/pull/44090)).
- `Mobile` updated to ignore `react/exhaustive-deps` eslint rule ([#44207](https://github.com/WordPress/gutenberg/pull/44207)).
- `RangeControl`: updated to pass `react/exhaustive-deps` eslint rule ([#44271](https://github.com/WordPress/gutenberg/pull/44271)).
- `UnitControl` updated to pass the `react/exhaustive-deps` eslint rule ([#44161](https://github.com/WordPress/gutenberg/pull/44161)).
- `Notice`: updated to satisfy `react/exhaustive-deps` eslint rule ([#44157](https://github.com/WordPress/gutenberg/pull/44157))
- `Popover`: refactor unit tests to TypeScript and modern RTL assertions ([#44373](https://github.com/WordPress/gutenberg/pull/44373)).

## 21.0.0 (2022-09-13)

Expand Down
70 changes: 59 additions & 11 deletions packages/components/src/popover/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,64 @@ type PlacementToInitialTranslationTuple = [
CSSProperties[ 'translate' ]
];

// There's no matching `placement` for 'middle center' positions,
// fallback to 'bottom' (same as `floating-ui`'s default.)
const FALLBACK_FOR_MIDDLE_CENTER_POSITIONS = 'bottom';

const ALL_POSITIONS_TO_EXPECTED_PLACEMENTS: PositionToPlacementTuple[] = [
// Format: [yAxis]
[ 'middle', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'bottom', 'bottom' ],
[ 'top', 'top' ],
// Format: [yAxis] [xAxis]
[ 'middle left', 'left' ],
[ 'middle center', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'middle right', 'right' ],
[ 'bottom left', 'bottom-end' ],
[ 'bottom center', 'bottom' ],
[ 'bottom right', 'bottom-start' ],
[ 'top left', 'top-end' ],
[ 'top center', 'top' ],
[ 'top right', 'top-start' ],
// Format: [yAxis] [xAxis] [corner]
[ 'middle left left', 'left' ],
[ 'middle left right', 'left' ],
[ 'middle left bottom', 'left-end' ],
[ 'middle left top', 'left-start' ],
[ 'middle center left', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'middle center right', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'middle center bottom', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'middle center top', FALLBACK_FOR_MIDDLE_CENTER_POSITIONS ],
[ 'middle right left', 'right' ],
[ 'middle right right', 'right' ],
[ 'middle right bottom', 'right-end' ],
[ 'middle right top', 'right-start' ],
[ 'bottom left left', 'bottom-end' ],
[ 'bottom left right', 'bottom-end' ],
[ 'bottom left bottom', 'bottom-end' ],
[ 'bottom left top', 'bottom-end' ],
[ 'bottom center left', 'bottom' ],
[ 'bottom center right', 'bottom' ],
[ 'bottom center bottom', 'bottom' ],
[ 'bottom center top', 'bottom' ],
[ 'bottom right left', 'bottom-start' ],
[ 'bottom right right', 'bottom-start' ],
[ 'bottom right bottom', 'bottom-start' ],
[ 'bottom right top', 'bottom-start' ],
[ 'top left left', 'top-end' ],
[ 'top left right', 'top-end' ],
[ 'top left bottom', 'top-end' ],
[ 'top left top', 'top-end' ],
[ 'top center left', 'top' ],
[ 'top center right', 'top' ],
[ 'top center bottom', 'top' ],
[ 'top center top', 'top' ],
[ 'top right left', 'top-start' ],
[ 'top right right', 'top-start' ],
[ 'top right bottom', 'top-start' ],
[ 'top right top', 'top-start' ],
];

describe( 'Popover', () => {
describe( 'Component', () => {
describe( 'basic behavior', () => {
Expand Down Expand Up @@ -93,17 +151,7 @@ describe( 'Popover', () => {
} );

describe( 'positionToPlacement', () => {
it.each( [
[ 'top left', 'top-end' ],
[ 'top center', 'top' ],
[ 'top right', 'top-start' ],
[ 'middle left', 'left' ],
[ 'middle center', 'center' ],
[ 'middle right', 'right' ],
[ 'bottom left', 'bottom-end' ],
[ 'bottom center', 'bottom' ],
[ 'bottom right', 'bottom-start' ],
] as PositionToPlacementTuple[] )(
it.each( ALL_POSITIONS_TO_EXPECTED_PLACEMENTS )(
'converts `%s` to `%s`',
( inputPosition, expectedPlacement ) => {
expect( positionToPlacement( inputPosition ) ).toEqual(
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/popover/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export type PopoverProps = {
* _Note: this prop is deprecated. Use the `placement` prop instead._
*/
position?:
| `${ PositionYAxis }`
| `${ PositionYAxis } ${ PositionXAxis }`
| `${ PositionYAxis } ${ PositionXAxis } ${ PositionCorner }`;
/**
Expand Down
74 changes: 58 additions & 16 deletions packages/components/src/popover/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,62 @@ import type {
PopoverAnchorRefTopBottom,
} from './types';

const POSITION_TO_PLACEMENT: Record<
NonNullable< PopoverProps[ 'position' ] >,
NonNullable< PopoverProps[ 'placement' ] >
> = {
bottom: 'bottom',
top: 'top',
'middle left': 'left',
'middle right': 'right',
'bottom left': 'bottom-end',
'bottom center': 'bottom',
'bottom right': 'bottom-start',
'top left': 'top-end',
'top center': 'top',
'top right': 'top-start',
'middle left left': 'left',
'middle left right': 'left',
'middle left bottom': 'left-end',
'middle left top': 'left-start',
'middle right left': 'right',
'middle right right': 'right',
'middle right bottom': 'right-end',
'middle right top': 'right-start',
'bottom left left': 'bottom-end',
'bottom left right': 'bottom-end',
'bottom left bottom': 'bottom-end',
'bottom left top': 'bottom-end',
'bottom center left': 'bottom',
'bottom center right': 'bottom',
'bottom center bottom': 'bottom',
'bottom center top': 'bottom',
'bottom right left': 'bottom-start',
'bottom right right': 'bottom-start',
'bottom right bottom': 'bottom-start',
'bottom right top': 'bottom-start',
'top left left': 'top-end',
'top left right': 'top-end',
'top left bottom': 'top-end',
'top left top': 'top-end',
'top center left': 'top',
'top center right': 'top',
'top center bottom': 'top',
'top center top': 'top',
'top right left': 'top-start',
'top right right': 'top-start',
'top right bottom': 'top-start',
'top right top': 'top-start',
// `middle`/`middle center [corner?]` positions are associated to a fallback
// `bottom` placement because there aren't any corresponding placement values.
middle: 'bottom',
'middle center': 'bottom',
'middle center bottom': 'bottom',
'middle center left': 'bottom',
'middle center right': 'bottom',
'middle center top': 'bottom',
};

/**
* Converts the `Popover`'s legacy "position" prop to the new "placement" prop
* (used by `floating-ui`).
Expand All @@ -23,22 +79,8 @@ import type {
*/
export const positionToPlacement = (
position: NonNullable< PopoverProps[ 'position' ] >
): NonNullable< PopoverProps[ 'placement' ] > => {
const [ x, y, z ] = position.split( ' ' );

if ( [ 'top', 'bottom' ].includes( x ) ) {
let suffix = '';
if ( ( !! z && z === 'left' ) || y === 'right' ) {
suffix = '-start';
} else if ( ( !! z && z === 'right' ) || y === 'left' ) {
suffix = '-end';
}

return ( x + suffix ) as NonNullable< PopoverProps[ 'placement' ] >;
}

return y as NonNullable< PopoverProps[ 'placement' ] >;
};
): NonNullable< PopoverProps[ 'placement' ] > =>
POSITION_TO_PLACEMENT[ position ] ?? 'bottom';

/**
* @typedef AnimationOrigin
Expand Down