chore: Fix funnel support for modals (#1494)

cloudscape-design · Sep 14, 2023 · 6d19d70 · 6d19d70
1 parent d8b1fd0
commit 6d19d70
Show file tree

Hide file tree

Showing 13 changed files with 174 additions and 50 deletions.
diff --git a/src/cards/index.tsx b/src/cards/index.tsx
@@ -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';
@@ -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
@@ -190,7 +190,7 @@ const Cards = React.forwardRef(function <T = any>(
                 />
               )}
             </div>
-          </InternalContainer>
+          </InternalContainerAsSubstep>
         </div>
       </AnalyticsFunnelSubStep>
     </LinkDefaultVariantContext.Provider>

diff --git a/src/container/__tests__/analytics.test.tsx b/src/container/__tests__/analytics.test.tsx
@@ -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';
@@ -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}>

diff --git a/src/container/index.tsx b/src/container/index.tsx
@@ -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';
@@ -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}
       />

diff --git a/src/container/internal.tsx b/src/container/internal.tsx
@@ -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({
@@ -52,6 +60,8 @@ export default function InternalContainer({
   __headerRef,
   __darkHeader = false,
   __disableStickyMobile = true,
+  __funnelSubStepProps,
+  __subStepRef,
   ...restProps
 }: InternalContainerProps) {
   const isMobile = useMobile();
@@ -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);
 
   /**
@@ -104,7 +113,7 @@ export default function InternalContainer({
   return (
     <div
       {...baseProps}
-      {...funnelSubStepProps}
+      {...__funnelSubStepProps}
       className={clsx(
         baseProps.className,
         styles.root,

diff --git a/src/expandable-section/expandable-section-container.tsx b/src/expandable-section/expandable-section-container.tsx
@@ -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 = ({
@@ -22,6 +24,8 @@ export const ExpandableSectionContainer = ({
   expanded,
   disableContentPaddings,
   __internalRootRef,
+  __funnelSubStepProps,
+  __subStepRef,
   ...rest
 }: ExpandableSectionContainerProps) => {
   if (variant === 'container' || variant === 'stacked') {
@@ -35,6 +39,8 @@ export const ExpandableSectionContainer = ({
         disableHeaderPaddings={true}
         __hiddenContent={!expanded}
         __internalRootRef={__internalRootRef}
+        __funnelSubStepProps={__funnelSubStepProps}
+        __subStepRef={__subStepRef}
       >
         {children}
       </InternalContainer>

diff --git a/src/expandable-section/index.tsx b/src/expandable-section/index.tsx
@@ -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');
diff --git a/src/expandable-section/internal.tsx b/src/expandable-section/internal.tsx
@@ -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,
@@ -36,6 +38,8 @@ export default function InternalExpandableSection({
   disableContentPaddings,
   headerAriaLabel,
   __internalRootRef,
+  __funnelSubStepProps,
+  __subStepRef,
   ...props
 }: InternalExpandableSectionProps) {
   const ref = useRef<HTMLDivElement>(null);
@@ -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

diff --git a/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx b/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx
@@ -17,6 +17,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';
@@ -326,10 +328,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>
       </>
@@ -346,19 +349,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,
         },
       ],
@@ -434,6 +437,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';

diff --git a/src/internal/analytics/components/analytics-funnel.tsx b/src/internal/analytics/components/analytics-funnel.tsx
@@ -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';
 
@@ -369,7 +370,7 @@ const InnerAnalyticsFunnelStep = ({ children, stepNumber, stepNameSelector }: An
   );
 };
 interface AnalyticsFunnelSubStepProps {
-  children?: React.ReactNode;
+  children?: React.ReactNode | ((props: FunnelSubStepContextValue) => React.ReactNode);
 }
 
 export const AnalyticsFunnelSubStep = ({ children }: AnalyticsFunnelSubStepProps) => {
@@ -415,6 +416,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 () => {
@@ -432,7 +437,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;
 
         /*
@@ -456,7 +461,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>
+  );
 };
diff --git a/src/internal/analytics/hooks/use-funnel.ts b/src/internal/analytics/hooks/use-funnel.ts
@@ -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.
@@ -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') {