Skip to content

Commit

Permalink
[EuiSelectableTemplateSitewide] Fix React wrappers breaking `popoverB…
Browse files Browse the repository at this point in the history
…utton` behavior (#7339)
  • Loading branch information
cee-chen authored Nov 3, 2023
1 parent b02248c commit 92cef86
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,80 +51,22 @@ exports[`EuiSelectableTemplateSitewide is rendered 1`] = `
</div>
`;

exports[`EuiSelectableTemplateSitewide props popoverButton is not rendered with popoverButtonBreakpoints xs 1`] = `
<div
class="euiSelectable euiSelectableTemplateSitewide"
>
<div
class="euiPopover emotion-euiPopover-block"
>
<div
class="euiFormControlLayout euiFormControlLayout--fullWidth"
>
<div
class="euiFormControlLayout__childrenWrapper"
>
<div
class="euiFormControlLayoutIcons euiFormControlLayoutIcons--left euiFormControlLayoutIcons--absolute"
>
<span
class="euiFormControlLayoutCustomIcon"
>
<span
aria-hidden="true"
class="euiFormControlLayoutCustomIcon__icon"
data-euiicon-type="search"
/>
</span>
</div>
<input
aria-activedescendant=""
aria-describedby="generated-id_instructions"
aria-haspopup="listbox"
aria-label="Filter options"
autocomplete="off"
class="euiFieldSearch euiFieldSearch--fullWidth euiSelectableSearch euiSelectableTemplateSitewide__search"
placeholder="Search for anything..."
type="search"
value=""
/>
</div>
</div>
<p
class="emotion-euiScreenReaderOnly"
id="generated-id_instructions"
>
Use the Up and Down arrow keys to move focus over options. Press Enter to select. Press Escape to collapse options.
</p>
</div>
</div>
`;

exports[`EuiSelectableTemplateSitewide props popoverButton is rendered 1`] = `
<div
class="euiSelectable euiSelectableTemplateSitewide"
>
<div
class="euiPopover emotion-euiPopover-inline-block"
>
<button>
Button
</button>
</div>
</div>
`;

exports[`EuiSelectableTemplateSitewide props popoverButton is rendered with popoverButtonBreakpoints m 1`] = `
<div
class="euiSelectable euiSelectableTemplateSitewide"
>
<div
class="euiPopover emotion-euiPopover-inline-block"
>
<button>
Button
</button>
<span
class="euiSelectableTemplateSitewide__popoverTrigger"
>
<button
data-test-subj="mobilePopoverButton"
>
Button
</button>
</span>
</div>
</div>
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
*/

import React from 'react';
import { fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { render, waitForEuiPopoverOpen } from '../../../test/rtl';
import { shouldRenderCustomStyles } from '../../../test/internal';
import { requiredProps } from '../../../test/required_props';
import { render } from '../../../test/rtl';

import { EuiSelectableTemplateSitewide } from './selectable_template_sitewide';
import { EuiSelectableTemplateSitewideOption } from './selectable_template_sitewide_option';
Expand Down Expand Up @@ -127,39 +129,73 @@ describe('EuiSelectableTemplateSitewide', () => {
beforeAll(() => (window.innerWidth = 670));
afterAll(() => 1024); // reset to jsdom's default

const button = (
<button data-test-subj="mobilePopoverButton">Button</button>
);

test('is rendered', () => {
const { container } = render(
<EuiSelectableTemplateSitewide
options={options}
popoverButton={<button>Button</button>}
popoverButton={button}
/>
);

expect(container.firstChild).toMatchSnapshot();
});

test('is rendered with popoverButtonBreakpoints m', () => {
const { container } = render(
const { getByTestSubject } = render(
<EuiSelectableTemplateSitewide
options={options}
popoverButton={<button>Button</button>}
popoverButton={button}
popoverButtonBreakpoints={['xs', 's', 'm']}
/>
);

expect(container.firstChild).toMatchSnapshot();
expect(getByTestSubject('mobilePopoverButton')).toBeInTheDocument();
});

test('is not rendered with popoverButtonBreakpoints xs', () => {
const { container } = render(
const { queryByTestSubject } = render(
<EuiSelectableTemplateSitewide
options={options}
popoverButton={<button>Button</button>}
popoverButton={button}
popoverButtonBreakpoints={['xs']}
/>
);

expect(container.firstChild).toMatchSnapshot();
expect(
queryByTestSubject('mobilePopoverButton')
).not.toBeInTheDocument();
});

test('toggles the selectable popover for keyboard users', () => {
const { getByTestSubject } = render(
<EuiSelectableTemplateSitewide
options={options}
popoverButton={<div>{button}</div>}
/>
);
// fireEvent doesn't seem to work: https://github.com/testing-library/user-event/issues/179#issuecomment-1125146667
getByTestSubject('mobilePopoverButton').focus();
userEvent.keyboard('{enter}');
waitForEuiPopoverOpen();

expect(getByTestSubject('euiSelectableList')).toBeInTheDocument();
});

test('toggles the selectable popover even when a wrapper exists around the button', () => {
const { getByTestSubject } = render(
<EuiSelectableTemplateSitewide
options={options}
popoverButton={<>{button}</>}
/>
);
fireEvent.click(getByTestSubject('mobilePopoverButton'));
waitForEuiPopoverOpen();

expect(getByTestSubject('euiSelectableList')).toBeInTheDocument();
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import React, {
ReactNode,
useState,
useMemo,
useCallback,
CSSProperties,
ReactElement,
} from 'react';
Expand Down Expand Up @@ -106,9 +107,9 @@ export const EuiSelectableTemplateSitewide: FunctionComponent<
_closePopover && _closePopover();
};

const togglePopover = () => {
setPopoverIsOpen(!popoverIsOpen);
};
const togglePopover = useCallback(() => {
setPopoverIsOpen((isOpen) => !isOpen);
}, []);

// Width applied to the internal div
const popoverWidth: CSSProperties['width'] = width || 600;
Expand Down Expand Up @@ -195,17 +196,18 @@ export const EuiSelectableTemplateSitewide: FunctionComponent<
return popoverButtonBreakpoints.includes(currentBreakpoint);
}, [currentBreakpoint, popoverButtonBreakpoints]);

let popoverTrigger: ReactElement;
if (popoverButton && canShowPopoverButton) {
popoverTrigger = React.cloneElement(popoverButton, {
...popoverButton.props,
onClick: togglePopover,
onKeyDown: (e: KeyboardEvent) => {
// Selectable preventsDefault on Enter which kills browser controls for pressing the button
e.stopPropagation();
},
});
}
const popoverTrigger = useMemo(() => {
if (!popoverButton || !canShowPopoverButton) return;
return (
<span
className="euiSelectableTemplateSitewide__popoverTrigger"
onClick={togglePopover}
onKeyDown={(e) => e.stopPropagation()}
>
{popoverButton}
</span>
);
}, [popoverButton, canShowPopoverButton, togglePopover]);

return (
<EuiSelectable
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/7339.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed an `EuiSelectableTemplateSitewide` bug where the `popoverButton` behavior would break if passed a non-DOM React wrapper

0 comments on commit 92cef86

Please sign in to comment.