Skip to content

Commit

Permalink
chore: Fix funnel support for modals
Browse files Browse the repository at this point in the history
  • Loading branch information
connorlanigan committed Sep 13, 2023
1 parent 11090e4 commit fc8af32
Show file tree
Hide file tree
Showing 13 changed files with 174 additions and 50 deletions.
6 changes: 3 additions & 3 deletions src/cards/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
useSelectionFocusMove,
useSelection,
} from '../table/selection';
import InternalContainer from '../container/internal';
import { InternalContainerAsSubstep } from '../container/internal';
import InternalStatusIndicator from '../status-indicator/internal';
import { applyDisplayName } from '../internal/utils/apply-display-name';
import stickyScrolling from '../table/sticky-scrolling';
Expand Down Expand Up @@ -139,7 +139,7 @@ const Cards = React.forwardRef(function <T = any>(
<LinkDefaultVariantContext.Provider value={{ defaultVariant: 'primary' }}>
<AnalyticsFunnelSubStep>
<div {...baseProps} className={clsx(baseProps.className, styles.root)} ref={mergedRef}>
<InternalContainer
<InternalContainerAsSubstep
header={
hasToolsHeader && (
<div
Expand Down Expand Up @@ -190,7 +190,7 @@ const Cards = React.forwardRef(function <T = any>(
/>
)}
</div>
</InternalContainer>
</InternalContainerAsSubstep>
</div>
</AnalyticsFunnelSubStep>
</LinkDefaultVariantContext.Provider>
Expand Down
29 changes: 29 additions & 0 deletions src/container/__tests__/analytics.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from 'react';
import { render, act } from '@testing-library/react';

import Container from '../../../lib/components/container';
import Modal from '../../../lib/components/modal';

import { FunnelMetrics } from '../../../lib/components/internal/analytics';
import { DATA_ATTR_FUNNEL_SUBSTEP } from '../../../lib/components/internal/analytics/selectors';
Expand Down Expand Up @@ -118,6 +119,34 @@ describe('Funnel Analytics', () => {
expect(FunnelMetrics.funnelSubStepComplete).not.toHaveBeenCalled();
});

test('Modal containers do not send their own events', async () => {
const { getByTestId } = render(
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
<AnalyticsFunnelStep stepNumber={2} stepNameSelector=".step-name-selector">
<Container>
<input data-testid="input-one" />

<Modal visible={true}>
<input data-testid="input-two" />
</Modal>
</Container>
</AnalyticsFunnelStep>
</AnalyticsFunnel>
);
act(() => void jest.runAllTimers());

expect(FunnelMetrics.funnelSubStepStart).not.toHaveBeenCalled();

getByTestId('input-one').focus();
getByTestId('input-two').focus();
getByTestId('input-one').focus();

await runPendingPromises();

expect(FunnelMetrics.funnelSubStepStart).toHaveBeenCalledTimes(1);
expect(FunnelMetrics.funnelSubStepComplete).not.toHaveBeenCalled();
});

test('sibling containers send their own events', async () => {
const { getByTestId } = render(
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
Expand Down
8 changes: 5 additions & 3 deletions src/container/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import InternalContainer from './internal';
import { InternalContainerAsSubstep } from './internal';
import { ContainerProps } from './interfaces';
import { getExternalProps } from '../internal/utils/external-props';
import { applyDisplayName } from '../internal/utils/apply-display-name';
Expand All @@ -18,12 +18,14 @@ export default function Container({
}: ContainerProps) {
const baseComponentProps = useBaseComponent('Container');
const externalProps = getExternalProps(props);

return (
<AnalyticsFunnelSubStep>
<InternalContainer
<InternalContainerAsSubstep
variant={variant}
disableHeaderPaddings={disableHeaderPaddings}
disableContentPaddings={disableContentPaddings}
disableHeaderPaddings={disableHeaderPaddings}
{...props}
{...externalProps}
{...baseComponentProps}
/>
Expand Down
15 changes: 12 additions & 3 deletions src/container/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ export interface InternalContainerProps extends Omit<ContainerProps, 'variant'>,
* * `full-page` – Only for internal use in table, cards and other components
*/
variant?: ContainerProps['variant'] | 'embedded' | 'full-page' | 'cards';

__funnelSubStepProps?: ReturnType<typeof useFunnelSubStep>['funnelSubStepProps'];
__subStepRef?: ReturnType<typeof useFunnelSubStep>['subStepRef'];
}

export function InternalContainerAsSubstep(props: InternalContainerProps) {
const { subStepRef, funnelSubStepProps } = useFunnelSubStep();
return <InternalContainer {...props} __subStepRef={subStepRef} __funnelSubStepProps={funnelSubStepProps} />;
}

export default function InternalContainer({
Expand All @@ -52,6 +60,8 @@ export default function InternalContainer({
__headerRef,
__darkHeader = false,
__disableStickyMobile = true,
__funnelSubStepProps,
__subStepRef,
...restProps
}: InternalContainerProps) {
const isMobile = useMobile();
Expand All @@ -68,12 +78,11 @@ export default function InternalContainer({
);
const { setHasStickyBackground } = useAppLayoutContext();
const isRefresh = useVisualRefresh();
const { subStepRef, funnelSubStepProps } = useFunnelSubStep();

const hasDynamicHeight = isRefresh && variant === 'full-page';
const overlapElement = useDynamicOverlap({ disabled: !hasDynamicHeight || !__darkHeader });

const mergedRef = useMergeRefs(rootRef, subStepRef, __internalRootRef);
const mergedRef = useMergeRefs(rootRef, __subStepRef, __internalRootRef);
const headerMergedRef = useMergeRefs(headerRef, overlapElement, __headerRef);

/**
Expand Down Expand Up @@ -104,7 +113,7 @@ export default function InternalContainer({
return (
<div
{...baseProps}
{...funnelSubStepProps}
{...__funnelSubStepProps}
className={clsx(
baseProps.className,
styles.root,
Expand Down
10 changes: 8 additions & 2 deletions src/expandable-section/expandable-section-container.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import InternalContainer from '../container/internal';
import InternalContainer, { InternalContainerProps } from '../container/internal';
import React from 'react';
import { ExpandableSectionProps } from './interfaces';
import { InternalBaseComponentProps } from '../internal/hooks/use-base-component';

interface ExpandableSectionContainerProps extends InternalBaseComponentProps {
export interface ExpandableSectionContainerProps extends InternalBaseComponentProps {
className?: string;
header: React.ReactNode;
children?: React.ReactNode;
variant: ExpandableSectionProps.Variant;
expanded: boolean | undefined;
disableContentPaddings: boolean | undefined;
__funnelSubStepProps?: InternalContainerProps['__funnelSubStepProps'];
__subStepRef?: InternalContainerProps['__subStepRef'];
}

export const ExpandableSectionContainer = ({
Expand All @@ -22,6 +24,8 @@ export const ExpandableSectionContainer = ({
expanded,
disableContentPaddings,
__internalRootRef,
__funnelSubStepProps,
__subStepRef,
...rest
}: ExpandableSectionContainerProps) => {
if (variant === 'container' || variant === 'stacked') {
Expand All @@ -35,6 +39,8 @@ export const ExpandableSectionContainer = ({
disableHeaderPaddings={true}
__hiddenContent={!expanded}
__internalRootRef={__internalRootRef}
__funnelSubStepProps={__funnelSubStepProps}
__subStepRef={__subStepRef}
>
{children}
</InternalContainer>
Expand Down
19 changes: 14 additions & 5 deletions src/expandable-section/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,31 @@ import React from 'react';

import { ExpandableSectionProps } from './interfaces';
import { applyDisplayName } from '../internal/utils/apply-display-name';
import InternalExpandableSection from './internal';
import InternalExpandableSection, { InternalExpandableSectionProps } from './internal';
import useBaseComponent from '../internal/hooks/use-base-component';
import { AnalyticsFunnelSubStep } from '../internal/analytics/components/analytics-funnel';
import { useFunnelSubStep } from '../internal/analytics/hooks/use-funnel';

export { ExpandableSectionProps };

export default function ExpandableSection({ variant = 'default', ...props }: ExpandableSectionProps) {
const baseComponentProps = useBaseComponent('ExpandableSection');

const expandableSection = <InternalExpandableSection variant={variant} {...props} {...baseComponentProps} />;

if (variant === 'container' || variant === 'stacked') {
return <AnalyticsFunnelSubStep>{expandableSection}</AnalyticsFunnelSubStep>;
return (
<AnalyticsFunnelSubStep>
<InternalExpandableSectionAsSubstep variant={variant} {...props} {...baseComponentProps} />
</AnalyticsFunnelSubStep>
);
} else {
return expandableSection;
return <InternalExpandableSection variant={variant} {...props} {...baseComponentProps} />;
}
}

function InternalExpandableSectionAsSubstep(props: InternalExpandableSectionProps) {
const { subStepRef, funnelSubStepProps } = useFunnelSubStep();

return <InternalExpandableSection {...props} __subStepRef={subStepRef} __funnelSubStepProps={funnelSubStepProps} />;
}

applyDisplayName(ExpandableSection, 'ExpandableSection');
10 changes: 8 additions & 2 deletions src/expandable-section/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ import { fireNonCancelableEvent } from '../internal/events';
import { ExpandableSectionProps } from './interfaces';

import styles from './styles.css.js';
import { ExpandableSectionContainer } from './expandable-section-container';
import { ExpandableSectionContainer, ExpandableSectionContainerProps } from './expandable-section-container';
import { ExpandableSectionHeader } from './expandable-section-header';
import { InternalBaseComponentProps } from '../internal/hooks/use-base-component';
import { variantSupportsDescription } from './utils';

type InternalExpandableSectionProps = ExpandableSectionProps & InternalBaseComponentProps;
export type InternalExpandableSectionProps = ExpandableSectionProps &
InternalBaseComponentProps &
Pick<ExpandableSectionContainerProps, '__funnelSubStepProps' | '__subStepRef'>;

export default function InternalExpandableSection({
expanded: controlledExpanded,
Expand All @@ -36,6 +38,8 @@ export default function InternalExpandableSection({
disableContentPaddings,
headerAriaLabel,
__internalRootRef,
__funnelSubStepProps,
__subStepRef,
...props
}: InternalExpandableSectionProps) {
const ref = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -98,6 +102,8 @@ export default function InternalExpandableSection({
expanded={expanded}
className={clsx(baseProps.className, styles.root)}
variant={variant}
__funnelSubStepProps={__funnelSubStepProps}
__subStepRef={__subStepRef}
disableContentPaddings={disableContentPaddings}
header={
<ExpandableSectionHeader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import FormField from '../../../../../lib/components/form-field';
import Container from '../../../../../lib/components/container';
import Cards from '../../../../../lib/components/cards';
import Table from '../../../../../lib/components/table';
import Header from '../../../../../lib/components/header';
import Modal from '../../../../../lib/components/modal';
import ExpandableSection from '../../../../../lib/components/expandable-section';

import { mockedFunnelInteractionId, mockFunnelMetrics, mockInnerText } from '../mocks';
Expand Down Expand Up @@ -325,10 +327,11 @@ describe('AnalyticsFunnelStep', () => {
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
<AnalyticsFunnelStep stepNumber={stepNumber} stepNameSelector={stepNameSelector}>
Step Content
<Container />
<Cards items={[]} cardDefinition={{}} />
<Table items={[]} columnDefinitions={[]} />
<ExpandableSection variant="container" />
<Container header={<Header>Container</Header>} />
<Cards header={<Header>Cards</Header>} items={[]} cardDefinition={{}} />
<Table header={<Header>Table</Header>} items={[]} columnDefinitions={[]} />
<ExpandableSection headerText="ExpandableSection" variant="container" />
<Modal visible={true}></Modal>
</AnalyticsFunnelStep>
</AnalyticsFunnel>
</>
Expand All @@ -345,19 +348,19 @@ describe('AnalyticsFunnelStep', () => {
totalSubSteps: 4,
subStepConfiguration: [
{
name: '',
name: 'Container',
number: 1,
},
{
name: '',
name: 'Cards',
number: 2,
},
{
name: '',
name: 'Table',
number: 3,
},
{
name: '',
name: 'ExpandableSection',
number: 4,
},
],
Expand Down Expand Up @@ -433,6 +436,23 @@ describe('AnalyticsFunnelStep', () => {
);
});

test('does not treat Modals as their own substep', () => {
render(
<AnalyticsFunnel funnelType="single-page" optionalStepNumbers={[]} totalFunnelSteps={1}>
<AnalyticsFunnelStep stepNumber={1} stepNameSelector={''}>
<Modal visible={true}></Modal>
</AnalyticsFunnelStep>
</AnalyticsFunnel>
);
act(() => void jest.runAllTimers());

expect(FunnelMetrics.funnelStepStart).toHaveBeenCalledWith(
expect.objectContaining({
totalSubSteps: 0,
})
);
});

test('does not call funnelStepComplete when the funnel unmounts without submitting', () => {
const stepNumber = 1;
const stepNameSelector = '.step-name-selector';
Expand Down
17 changes: 14 additions & 3 deletions src/internal/analytics/components/analytics-funnel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
getSubStepSelector,
} from '../selectors';
import { useDebounceCallback } from '../../hooks/use-debounce-callback';
import { nodeBelongs } from '../../utils/node-belongs';

export const FUNNEL_VERSION = '1.2';

Expand Down Expand Up @@ -358,7 +359,7 @@ const InnerAnalyticsFunnelStep = ({ children, stepNumber, stepNameSelector }: An
);
};
interface AnalyticsFunnelSubStepProps {
children?: React.ReactNode;
children?: React.ReactNode | ((props: FunnelSubStepContextValue) => React.ReactNode);
}

export const AnalyticsFunnelSubStep = ({ children }: AnalyticsFunnelSubStepProps) => {
Expand Down Expand Up @@ -404,6 +405,10 @@ export const AnalyticsFunnelSubStep = ({ children }: AnalyticsFunnelSubStepProps
const context = isNested ? inheritedContext : newContext;

useEffect(() => {
if (isNested || !subStepRef.current) {
return;
}

const onMouseDown = () => (mousePressed.current = true);

const onMouseUp = async () => {
Expand All @@ -421,7 +426,7 @@ export const AnalyticsFunnelSubStep = ({ children }: AnalyticsFunnelSubStepProps
*/
await new Promise(r => setTimeout(r, 1));

if (!subStepRef.current || !subStepRef.current.contains(document.activeElement)) {
if (!subStepRef.current || !document.activeElement || !nodeBelongs(subStepRef.current, document.activeElement)) {
isFocusedSubStep.current = false;

/*
Expand All @@ -445,7 +450,13 @@ export const AnalyticsFunnelSubStep = ({ children }: AnalyticsFunnelSubStepProps
subStepNameSelector,
subStepSelector,
focusCleanupFunction,
isNested,
subStepRef,
]);

return <FunnelSubStepContext.Provider value={context}>{children}</FunnelSubStepContext.Provider>;
return (
<FunnelSubStepContext.Provider value={context}>
{typeof children === 'function' ? children(context) : children}
</FunnelSubStepContext.Provider>
);
};
3 changes: 2 additions & 1 deletion src/internal/analytics/hooks/use-funnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getSubStepAllSelector,
} from '../selectors';
import { FunnelMetrics } from '../';
import { nodeBelongs } from '../../utils/node-belongs';

/**
* Custom React Hook to manage and interact with FunnelSubStep.
Expand Down Expand Up @@ -121,7 +122,7 @@ export const useFunnelSubStep = () => {
return;
}

if (!subStepRef.current || !subStepRef.current.contains(event.relatedTarget) || !event.relatedTarget) {
if (!subStepRef.current || !event.relatedTarget || !nodeBelongs(subStepRef.current, event.relatedTarget)) {
isFocusedSubStep.current = false;

if (funnelInteractionId && subStepId && funnelState.current !== 'cancelled') {
Expand Down
Loading

0 comments on commit fc8af32

Please sign in to comment.