Skip to content

Commit

Permalink
Merge branch 'main' into react-19-upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewgallo authored Dec 12, 2024
2 parents a61853e + 36bd4f9 commit 072d347
Show file tree
Hide file tree
Showing 13 changed files with 159 additions and 124 deletions.
22 changes: 0 additions & 22 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,28 +182,6 @@ jobs:
name: playwright-avt-report
path: .playwright

vrt-runner:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Setup Node.js
uses: actions/setup-node@v2
with:
node-version: '20.x'
cache: yarn
- name: Install
run: yarn
- name: Install browsers
run: yarn playwright install --with-deps
- name: Build project
run: yarn build
- name: Run VRT
working-directory: packages/core
env:
PERCY_TOKEN: web_d04495b0b413d61c2ea6b9118d1748b43f4fdd58d17ebe453ef8e0016b5766e4
run: yarn percy storybook storybook-static

avt:
if: ${{ always() }}
runs-on: ubuntu-latest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,18 @@ const children = `hello, world (${uuidv4()})`;
const dataTestId = uuidv4();
const className = `class-${uuidv4()}`;

const childrenContent = (
const childrenContent = [
<CoachmarkOverlayElement
key="element1"
title="Hello World"
description="this is a description test"
/>
);
/>,
<CoachmarkOverlayElement
key="element2"
title="Hello World"
description="this is a another description test"
/>,
];

const renderCoachmarkWithOverlayElements = (
{ ...rest } = {},
Expand Down Expand Up @@ -165,4 +171,22 @@ describe(componentName, () => {

expect(screen.getByRole('img')).toBeInTheDocument();
});

it('calls onNext', async () => {
const user = userEvent.setup();
const onNext = jest.fn();
renderCoachmarkWithOverlayElements({
'data-testid': dataTestId,
onNext,
});
const beaconOrButton = screen.getByRole('button', {
name: 'Show information',
});
await act(() => user.click(beaconOrButton));
const nextButton = screen.getByRole('button', {
name: 'Next',
});
await act(() => user.click(nextButton));
await expect(onNext).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ export interface CoachmarkOverlayElementsProps {
* The label for the Close button.
*/
closeButtonLabel?: string;
/**
* Callback called when clicking on the Next button.
*/
onNext?: () => void;
/**
* Callback called when clicking on the Previous button.
*/
onBack?: () => void;
/**
* Current step of the coachmarks.
*/
currentStep?: number;
}

// NOTE: the component SCSS is not imported here: it is rolled up separately.
Expand All @@ -96,6 +108,9 @@ const defaults = {
nextButtonText: 'Next',
previousButtonLabel: 'Back',
closeButtonLabel: 'Got it',
onNext: undefined,
onBack: undefined,
currentStep: 0,
};
/**
* Composable container to allow for the displaying of CoachmarkOverlayElement
Expand All @@ -112,9 +127,12 @@ export let CoachmarkOverlayElements = React.forwardRef<
isVisible = defaults.isVisible,
media,
renderMedia,
currentStep = defaults.currentStep,
nextButtonText = defaults.nextButtonText,
previousButtonLabel = defaults.previousButtonLabel,
closeButtonLabel = defaults.closeButtonLabel,
onNext = defaults.onNext,
onBack = defaults.onBack,
// Collect any other property values passed in.
...rest
},
Expand All @@ -123,7 +141,7 @@ export let CoachmarkOverlayElements = React.forwardRef<
const buttonFocusRef = useRef<ButtonProps<any> | undefined>(undefined);
const scrollRef = useRef<CarouselProps | undefined>(undefined);
const [scrollPosition, setScrollPosition] = useState(0);
const [currentProgStep, _setCurrentProgStep] = useState(0);
const [currentProgStep, _setCurrentProgStep] = useState(currentStep);
const coachmark = useCoachmark();
const hasMedia = media || renderMedia;

Expand All @@ -145,6 +163,16 @@ export let CoachmarkOverlayElements = React.forwardRef<
[currentProgStep, renderMedia]
);

useEffect(() => {
// When current step is set by props
// scroll to the appropriate view on the carrousel
const targetStep = clamp(currentStep, progStepFloor, progStepCeil);

scrollRef?.current?.scrollToView?.(targetStep);
// Avoid circular call to this hook
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [currentStep]);

useEffect(() => {
// On mount, one of the two primary buttons ("next" or "close")
// will be rendered and must have focus. (a11y)
Expand Down Expand Up @@ -222,7 +250,6 @@ export let CoachmarkOverlayElements = React.forwardRef<
) : (
<>
<Carousel
disableArrowScroll
ref={scrollRef as RefObject<HTMLDivElement>}
onScroll={(scrollPercent) => {
setScrollPosition(scrollPercent);
Expand All @@ -248,6 +275,7 @@ export let CoachmarkOverlayElements = React.forwardRef<
);
scrollRef?.current?.scrollToView?.(targetStep);
setCurrentProgStep(targetStep);
onBack?.();
}}
>
{previousButtonLabel}
Expand All @@ -268,6 +296,7 @@ export let CoachmarkOverlayElements = React.forwardRef<
);
scrollRef?.current?.scrollToView?.(targetStep);
setCurrentProgStep(targetStep);
onNext?.();
}}
>
{nextButtonText}
Expand Down Expand Up @@ -320,6 +349,10 @@ CoachmarkOverlayElements.propTypes = {
* The label for the Close button.
*/
closeButtonLabel: PropTypes.string,
/**
* Current step of the coachmarks
*/
currentStep: PropTypes.number,
/**
* The visibility of CoachmarkOverlayElements is
* managed in the parent component.
Expand All @@ -344,6 +377,14 @@ CoachmarkOverlayElements.propTypes = {
* The label for the Next button.
*/
nextButtonText: PropTypes.string,
/**
* Optional callback called when clicking on the Previous button.
*/
onBack: PropTypes.func,
/**
* Optional callback called when clicking on the Next button.
*/
onNext: PropTypes.func,
/**
* The label for the Previous button.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ export let CreateTearsheet = forwardRef(
verticalPosition,
closeIconDescription: '',
}}
currentStep={currentStep}
>
<div className={`${blockClass}__content`} ref={contentRef}>
<Form aria-label={title}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@ const Template = (args) => {
version. Please migrate to{' '}
<a href="https://ibm-products.carbondesignsystem.com/?path=/docs/ibm-products-patterns-full-page-error-fullpageerror--docs">
FullPageError
</a>
.
</a>{' '}
by running{' '}
<code>
npx @carbon/upgrade migrate ibm-products-update-http-errors --write
</code>
</div>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export let HTTPError403 = React.forwardRef<HTMLDivElement, HTTPError403Props>(
/**@ts-ignore*/
HTTPError403.deprecated = {
level: 'warn',
details: `Please replace ${componentName} with FullPageError`,
details: `${componentName} is deprecated. Please migrate to FullPageError by running npx @carbon/upgrade migrate ibm-products-update-http-errors --write`,
};

// Return a placeholder if not released and not enabled by feature flag
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ const Template = (args) => {
version. Please migrate to{' '}
<a href="https://ibm-products.carbondesignsystem.com/?path=/docs/ibm-products-patterns-full-page-error-fullpageerror--docs">
FullPageError
</a>
.
</a>{' '}
by running{' '}
<code>
npx @carbon/upgrade migrate ibm-products-update-http-errors --write
</code>
</div>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export let HTTPError404 = React.forwardRef<HTMLDivElement, HTTPError404Props>(
/**@ts-ignore*/
HTTPError404.deprecated = {
level: 'warn',
details: `Please replace ${componentName} with FullPageError`,
details: `${componentName} is deprecated. Please migrate to FullPageError by running npx @carbon/upgrade migrate ibm-products-update-http-errors --write`,
};

// Return a placeholder if not released and not enabled by feature flag
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ const Template = (args) => {
version. Please migrate to{' '}
<a href="https://ibm-products.carbondesignsystem.com/?path=/docs/ibm-products-patterns-full-page-error-fullpageerror--docs">
FullPageError
</a>
.
</a>{' '}
by running{' '}
<code>
npx @carbon/upgrade migrate ibm-products-update-http-errors --write
</code>
</div>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export let HTTPErrorOther = React.forwardRef(
/**@ts-ignore*/
HTTPErrorOther.deprecated = {
level: 'warn',
details: `Please replace ${componentName} with FullPageError`,
details: `${componentName} is deprecated. Please migrate to FullPageError by running npx @carbon/upgrade migrate ibm-products-update-http-errors --write`,
};

// Return a placeholder if not released and not enabled by feature flag
Expand Down
83 changes: 49 additions & 34 deletions packages/ibm-products/src/components/Tearsheet/Tearsheet.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const componentNameCreateNarrow = CreateTearsheetNarrow.displayName;
const onClick = jest.fn();
const onCloseReturnsFalse = jest.fn(() => false);
const onCloseReturnsTrue = jest.fn(() => true);
const onBlur = jest.fn();

const createButton = `Create ${uuidv4()}`;
const actions = [
Expand Down Expand Up @@ -81,6 +82,41 @@ const navigation = (
);
const title = `Title of the ${uuidv4()} tearsheet`;

const mainText = 'Main content 1';
const inputId = 'stacked-input-1';

// eslint-disable-next-line react/prop-types
const DummyComponent = ({ props, open }) => {
const buttonRef = React.useRef(undefined);

return (
<>
<Button ref={buttonRef}>Open</Button>
<Tearsheet
{...{ ...props, closeIconDescription }}
{...{
open: open,
}}
hasCloseIcon={true}
onClose={onCloseReturnsTrue}
open={open}
selectorPrimaryFocus={`#${inputId}`}
launcherButtonRef={buttonRef}
>
<div className="tearsheet-stories__dummy-content-block">
{mainText}
<TextInput
id={inputId}
data-testid={inputId}
labelText="Enter an important value here"
onBlur={onBlur}
/>
</div>
</Tearsheet>
</>
);
};

// These are tests than apply to both Tearsheet and TearsheetNarrow
// and also (with extra props and omitting button tests) to CreateTearsheetNarrow
const commonTests = (Ts, name, props, testActions) => {
Expand Down Expand Up @@ -211,40 +247,6 @@ const commonTests = (Ts, name, props, testActions) => {
});

it('should return focus to the launcher button', async () => {
const mainText = 'Main content 1';
const inputId = 'stacked-input-1';

// eslint-disable-next-line react/prop-types
const DummyComponent = ({ open }) => {
const buttonRef = React.useRef(undefined);

return (
<>
<Button ref={buttonRef}>Open</Button>
<Ts
{...{ ...props, closeIconDescription }}
{...{
open: open,
}}
hasCloseIcon={true}
onClose={onCloseReturnsTrue}
open={open}
selectorPrimaryFocus={`#${inputId}`}
launcherButtonRef={buttonRef}
>
<div className="tearsheet-stories__dummy-content-block">
{mainText}
<TextInput
id={inputId}
data-testid={inputId}
labelText="Enter an important value here"
/>
</div>
</Ts>
</>
);
};

const { rerender, getByText, getByTestId } = render(
<DummyComponent open={true} />
);
Expand All @@ -269,6 +271,19 @@ const commonTests = (Ts, name, props, testActions) => {
await act(() => new Promise((resolve) => setTimeout(resolve, 0)));
expect(launchButtonEl).toHaveFocus();
});

it('should call onBlur only once', async () => {
const { getByTestId } = render(<DummyComponent open={true} />);

const inputEl = getByTestId(inputId);
const closeButton = screen.getByRole('button', {
name: closeIconDescription,
});

expect(inputEl).toHaveFocus();
await act(() => userEvent.click(closeButton));
expect(onBlur).toHaveBeenCalledTimes(1);
});
}

it('is visible when open is true', async () => {
Expand Down
Loading

0 comments on commit 072d347

Please sign in to comment.