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: convert unit tests to TypeScript and modern RTL assertions #44373

Merged
merged 2 commits into from
Sep 23, 2022
Merged

Popover: convert unit tests to TypeScript and modern RTL assertions #44373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72f864b
Popover: convert unit tests to TypeScript
ciampo Sep 22, 2022
7c038e5
CHANGELOG
ciampo Sep 22, 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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- `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
34 changes: 0 additions & 34 deletions packages/components/src/popover/test/__snapshots__/index.js.snap
View file
Open in desktop

This file was deleted.

126 changes: 72 additions & 54 deletions ...ages/components/src/popover/test/index.js → ...ges/components/src/popover/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* External dependencies
*/
import { act, render, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import type { CSSProperties } from 'react';

/**
* WordPress dependencies
Expand All @@ -11,69 +12,83 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import Popover from '../';

import { positionToPlacement, placementToMotionAnimationProps } from '../utils';
import Popover from '..';
import type { PopoverProps } from '../types';

type PositionToPlacementTuple = [
NonNullable< PopoverProps[ 'position' ] >,
NonNullable< PopoverProps[ 'placement' ] >
];
type PlacementToAnimationOriginTuple = [
NonNullable< PopoverProps[ 'placement' ] >,
number,
number
];
type PlacementToInitialTranslationTuple = [
NonNullable< PopoverProps[ 'placement' ] >,
'translateY' | 'translateX',
CSSProperties[ 'translate' ]
];

describe( 'Popover', () => {
describe( 'Component', () => {
afterEach( () => {
if ( document.activeElement ) {
document.activeElement.blur();
}
} );
Comment on lines -20 to -24
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not necessary anymore

describe( 'basic behavior', () => {
it( 'should render content', () => {
render( <Popover>Hello</Popover> );

it( 'should allow focus-on-open behavior to be disabled', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test has been split into two separate tests (should focus the popover by default when opened, should allow focus-on-open behavior to be disabled)

expect( document.activeElement ).toBe( document.body );
expect( screen.getByText( 'Hello' ) ).toBeInTheDocument();
} );

act( () => {
render( <Popover focusOnMount={ false } /> );
it( 'should forward additional props to portaled element', () => {
render( <Popover role="tooltip">Hello</Popover> );

jest.advanceTimersByTime( 1 );
expect( screen.getByRole( 'tooltip' ) ).toBeInTheDocument();
} );

expect( document.activeElement ).toBe( document.body );
} );

it( 'should render content', () => {
let result;
act( () => {
result = render( <Popover>Hello</Popover> );
} );
describe( 'anchor', () => {
it( 'should render correctly when anchor is provided', () => {
const PopoverWithAnchor = ( args: PopoverProps ) => {
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ anchor, setAnchor ] =
useState< HTMLParagraphElement | null >( null );

return (
<div>
<p ref={ setAnchor }>Anchor</p>
<Popover { ...args } anchor={ anchor } />
</div>
);
};

expect(
result.container.querySelector( 'span' )
).toMatchSnapshot();
} );
render(
<PopoverWithAnchor>Popover content</PopoverWithAnchor>
);

it( 'should pass additional props to portaled element', () => {
let result;
act( () => {
result = render( <Popover role="tooltip">Hello</Popover> );
expect(
screen.getByText( 'Popover content' )
).toBeInTheDocument();
} );

expect(
result.container.querySelector( 'span' )
).toMatchSnapshot();
} );

it( 'should render correctly when anchor is provided', () => {
const PopoverWithAnchor = ( args ) => {
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ anchor, setAnchor ] = useState( null );

return (
<div>
<p ref={ setAnchor }>Anchor</p>
<Popover { ...args } anchor={ anchor } />
</div>
);
};
describe( 'focus behavior', () => {
it( 'should focus the popover by default when opened', () => {
render( <Popover>Popover content</Popover> );

render( <PopoverWithAnchor>Popover content</PopoverWithAnchor> );
expect(
screen.getByText( 'Popover content' ).parentElement
).toHaveFocus();
} );

expect( screen.getByText( 'Popover content' ) ).toBeInTheDocument();
it( 'should allow focus-on-open behavior to be disabled', () => {
render(
<Popover focusOnMount={ false }>Popover content</Popover>
);

expect( document.body ).toHaveFocus();
} );
} );
} );

Expand All @@ -88,11 +103,14 @@ describe( 'Popover', () => {
[ 'bottom left', 'bottom-end' ],
[ 'bottom center', 'bottom' ],
[ 'bottom right', 'bottom-start' ],
] )( 'converts `%s` to `%s`', ( inputPosition, expectedPlacement ) => {
expect( positionToPlacement( inputPosition ) ).toEqual(
expectedPlacement
);
} );
] as PositionToPlacementTuple[] )(
'converts `%s` to `%s`',
( inputPosition, expectedPlacement ) => {
expect( positionToPlacement( inputPosition ) ).toEqual(
expectedPlacement
);
}
);
} );

describe( 'placementToMotionAnimationProps', () => {
Expand All @@ -110,7 +128,7 @@ describe( 'Popover', () => {
[ 'left', 1, 0.5 ],
[ 'left-start', 1, 0 ],
[ 'left-end', 1, 1 ],
] )(
] as PlacementToAnimationOriginTuple[] )(
'for the `%s` placement computes an animation origin of (%d, %d)',
( inputPlacement, expectedOriginX, expectedOriginY ) => {
expect(
Expand Down Expand Up @@ -140,7 +158,7 @@ describe( 'Popover', () => {
[ 'left', 'translateX', '2em' ],
[ 'left-start', 'translateX', '2em' ],
[ 'left-end', 'translateX', '2em' ],
] )(
] as PlacementToInitialTranslationTuple[] )(
'for the `%s` placement computes an initial `%s` of `%s',
(
inputPlacement,
Expand Down