Skip to content

Commit

Permalink
[NO JIRA][BpkPopover]: Update popover to attach to body outside of ta…
Browse files Browse the repository at this point in the history 
…rget elements (#3497)

[NO JIRA][BpkPopover]: Update popover to attach to body outside of target elements (#3497)

* Fix accessibility of component and inaccurate test

* Correcting propagation test
  • Loading branch information
@olliecurtis
olliecurtis authored Jun 24, 2024
1 parent d5923d8 commit 5da7be9
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 119 deletions.
18 changes: 10 additions & 8 deletions packages/bpk-component-popover/src/BpkPopover-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* limitations under the License.
*/

import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom';

import BpkPopover from './BpkPopover';
Expand Down Expand Up @@ -91,12 +91,14 @@ describe('BpkPopover', () => {
});

it('should render correctly with "closeButtonProps" provided', () => {
const {container} = render(
render(
<BpkPopover
id="my-popover"
onClose={() => null}
label="My popover"
labelAsTitle
closeButtonLabel="Close"
closeButtonIcon
target={<button type="button">My target</button>}
closeButtonProps={{ tabIndex: 0 }}
isOpen
Expand All @@ -105,7 +107,7 @@ describe('BpkPopover', () => {
</BpkPopover>,
);

expect(container.querySelector('[tabindex="0"]')).toBeVisible();
expect(screen.getByRole('button', { name: /Close/i, })).toHaveAttribute('tabindex');
});

it('should render correctly with "padded" attribute equal to false', () => {
Expand Down Expand Up @@ -142,7 +144,7 @@ describe('BpkPopover', () => {
</BpkPopover>,
);

const heading = container.querySelector('.bpk-popover__header');
const heading = container.getElementsByClassName('.bpk-popover__header');
expect(heading).toBeTruthy();
});

Expand All @@ -162,7 +164,7 @@ describe('BpkPopover', () => {
</BpkPopover>,
);

const actionButton = container.querySelector('.bpk-popover__action');
const actionButton = container.getElementsByClassName('.bpk-popover__action');
expect(actionButton).toBeTruthy();
expect(screen.getByText('Action')).toBeVisible();
});
Expand Down Expand Up @@ -199,7 +201,7 @@ describe('BpkPopover', () => {
event.afterStopPropagation = true;
});

const { getByRole } = render(
render(
<BpkPopover
id="my-popover"
onClose={jest.fn()}
Expand All @@ -208,13 +210,12 @@ describe('BpkPopover', () => {
labelAsTitle
closeButtonIcon
target={<button onClick={handleClick} type="button">My target</button>}
isOpen
>
My popover content
</BpkPopover>,
);

const button = getByRole('button', { name: 'My target' });
const button = screen.getByRole('button', { name: 'My target' });

const event = new MouseEvent('click', { bubbles: true });
jest.spyOn(event, 'stopPropagation');
Expand All @@ -224,6 +225,7 @@ describe('BpkPopover', () => {
expect(handleClick).toHaveBeenCalled();
expect(event.stopPropagation).toHaveBeenCalled();
expect(handleClick.mock.calls[0][0].afterStopPropagation).toBe(true);
expect(screen.getByText('My popover content')).toBeVisible();
});
});

Expand Down
234 changes: 123 additions & 111 deletions packages/bpk-component-popover/src/BpkPopover.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@
*/

import type { SyntheticEvent, ReactNode, ReactElement } from 'react';
import { useState, useEffect, useRef, cloneElement, isValidElement } from 'react';
import {
useState,
useEffect,
useRef,
cloneElement,
isValidElement,
} from 'react';

import {
useFloating,
Expand All @@ -27,12 +33,12 @@ import {
useDismiss,
useInteractions,
FloatingFocusManager,
FloatingPortal,
arrow,
FloatingArrow,
shift,
} from '@floating-ui/react';


import { surfaceHighlightDay } from '@skyscanner/bpk-foundations-web/tokens/base.es6';

// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
Expand Down Expand Up @@ -70,16 +76,16 @@ const bindEventSource = (
callback(event, { source });
};

type CloseButtonProps = (
{
/**
* @deprecated close button text is deprecated. Instead, please use `closeButtonIcon`, or you may opt not to render a close button at all.
*/
closeButtonText: string;
} | {
closeButtonText?: never;
}
);
type CloseButtonProps =
| {
/**
* @deprecated close button text is deprecated. Instead, please use `closeButtonIcon`, or you may opt not to render a close button at all.
*/
closeButtonText: string;
}
| {
closeButtonText?: never;
};

export type Props = CloseButtonProps & {
children: ReactNode;
Expand Down Expand Up @@ -127,7 +133,7 @@ const BpkPopover = ({

useEffect(() => {
if (!isOpen) {
setIsOpenState(false)
setIsOpenState(false);
}
}, [isOpen]);

Expand Down Expand Up @@ -187,117 +193,123 @@ const BpkPopover = ({
<>
{targetElement}
{isOpenState && (
<FloatingFocusManager context={context} modal={false}>
<div
className={getClassName('bpk-popover--container')}
ref={refs.setFloating}
style={floatingStyles}
{...getFloatingProps()}
>
<TransitionInitialMount
appearClassName={getClassName('bpk-popover--appear')}
appearActiveClassName={getClassName('bpk-popover--appear-active')}
transitionTimeout={200}
<FloatingPortal>
<FloatingFocusManager context={context}>
<div
className={getClassName('bpk-popover--container')}
ref={refs.setFloating}
style={floatingStyles}
{...getFloatingProps()}
>
<section
id={id}
tabIndex={-1}
role="dialog"
aria-labelledby={labelId}
className={classNames}
{...rest}
>
{showArrow && (
<FloatingArrow
ref={arrowRef}
context={context}
id={ARROW_ID}
className={getClassName('bpk-popover__arrow')}
role="presentation"
stroke={surfaceHighlightDay}
strokeWidth={strokeWidth}
/>
<TransitionInitialMount
appearClassName={getClassName('bpk-popover--appear')}
appearActiveClassName={getClassName(
'bpk-popover--appear-active',
)}
{labelAsTitle ? (
<header className={getClassName('bpk-popover__header')}>
<BpkText
tagName="h2"
transitionTimeout={200}
>
<section
id={id}
tabIndex={-1}
role="dialog"
aria-labelledby={labelId}
className={classNames}
{...rest}
>
{showArrow && (
<FloatingArrow
ref={arrowRef}
context={context}
id={ARROW_ID}
className={getClassName('bpk-popover__arrow')}
role="presentation"
stroke={surfaceHighlightDay}
strokeWidth={strokeWidth}
/>
)}
{labelAsTitle ? (
<header className={getClassName('bpk-popover__header')}>
<BpkText
tagName="h2"
id={labelId}
textStyle={TEXT_STYLES.label1}
>
{label}
</BpkText>
&nbsp;
{closeButtonIcon ? (
<BpkCloseButton
label={closeButtonText || closeButtonLabel}
onClick={(
event: SyntheticEvent<HTMLButtonElement>,
) => {
bindEventSource(
EVENT_SOURCES.CLOSE_BUTTON,
onClose,
event,
);
setIsOpenState(false);
}}
{...closeButtonProps}
/>
) : (
closeButtonText && (
<BpkButtonLink
onClick={(
event: SyntheticEvent<HTMLButtonElement>,
) => {
bindEventSource(
EVENT_SOURCES.CLOSE_LINK,
onClose,
event,
);
setIsOpenState(false);
}}
{...closeButtonProps}
>
{closeButtonText}
</BpkButtonLink>
)
)}
</header>
) : (
<span
id={labelId}
textStyle={TEXT_STYLES.label1}
className={getClassName('bpk-popover__label')}
>
{label}
</BpkText>
&nbsp;
{closeButtonIcon ? (
<BpkCloseButton
label={closeButtonText || closeButtonLabel}
</span>
)}
<div className={bodyClassNames}>{children}</div>
{actionText && onAction && (
<div className={getClassName('bpk-popover__action')}>
<BpkButtonLink onClick={onAction}>
{actionText}
</BpkButtonLink>
</div>
)}
{!labelAsTitle && closeButtonText && (
<footer className={getClassName('bpk-popover__footer')}>
<BpkButtonLink
onClick={(event: SyntheticEvent<HTMLButtonElement>) => {
bindEventSource(
EVENT_SOURCES.CLOSE_BUTTON,
EVENT_SOURCES.CLOSE_LINK,
onClose,
event,
);
setIsOpenState(false);
}}
{...closeButtonProps}
/>
) : (
closeButtonText && (
<BpkButtonLink
onClick={(event: SyntheticEvent<HTMLButtonElement>) => {
bindEventSource(
EVENT_SOURCES.CLOSE_LINK,
onClose,
event,
);
setIsOpenState(false);
}}
{...closeButtonProps}
>
{closeButtonText}
</BpkButtonLink>
)
)}
</header>
) : (
<span
id={labelId}
className={getClassName('bpk-popover__label')}
>
{label}
</span>
)}
<div className={bodyClassNames}>{children}</div>
{actionText && onAction && (
<div className={getClassName('bpk-popover__action')}>
<BpkButtonLink
onClick={onAction}
>
{actionText}
</BpkButtonLink>
</div>
)}
{!labelAsTitle && closeButtonText && (
<footer className={getClassName('bpk-popover__footer')}>
<BpkButtonLink
onClick={(event: SyntheticEvent<HTMLButtonElement>) => {
bindEventSource(
EVENT_SOURCES.CLOSE_LINK,
onClose,
event,
);
setIsOpenState(false);
}}
{...closeButtonProps}
>
{closeButtonText}
</BpkButtonLink>
</footer>
)}
</section>
</TransitionInitialMount>
</div>
</FloatingFocusManager>
>
{closeButtonText}
</BpkButtonLink>
</footer>
)}
</section>
</TransitionInitialMount>
</div>
</FloatingFocusManager>
</FloatingPortal>
)}
</>
);
Expand Down

0 comments on commit 5da7be9

Please sign in to comment.