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.

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

[EuiSelectableTemplateSitewide] Fix React wrappers breaking popoverButton behavior #7339

Merged
merged 6 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading