Flow props standardization (#398)

* Update Form to YAML props standard

* Exclude video props from YAML passthrough

* Banner no longer needs the as={null} pattern for YAML props

* Convert Announcement to use merged props, generalize default esc handler for Dialog in Flow, standardize dismissible prop handling across components

* Use an actual Announcement flow type in Storybook, remove Dialog from Form story.

Author: dfltr

apps/smithy/src/stories/Announcement/Announcement.stories.tsx

@@ -8,7 +8,7 @@ export default {
 export const Default = {
   args: {
     dismissible: true,
-    flowId: "flow_8Ybz7lMK",
+    flowId: "flow_vLivpwoH",
     modal: true,
     onDismiss: () => console.log("Dismissed"),
   },

apps/smithy/src/stories/Form/Form.stories.tsx

@@ -32,7 +32,7 @@ export const Default = {
     fieldTypes: {
       customTest: CustomStep,
     },
-    as: Dialog,
+    // as: Dialog,
     variables: {
       testVar: "hello world",
     },

packages/react/src/components/Announcement/index.tsx

@@ -19,114 +19,83 @@ export interface AnnouncementProps extends FlowPropsWithoutChildren, DialogProps
   defaultOpen?: boolean
 }
 
-const flowPropNames = [
-  'dismissible',
-  'flowId',
-  'forceMount',
-  'onComplete',
-  'onDismiss',
-  'onPrimary',
-  'onSecondary',
-  'variables',
-]
-
 export function Announcement({ flowId, part, ...props }: AnnouncementProps) {
-  const flowProps = Object.fromEntries(
-    Object.entries(props).filter(([k]) => flowPropNames.some((name) => k === name))
-  )
-  const dialogProps = Object.fromEntries(
-    Object.entries(props).filter(([k]) => flowPropNames.indexOf(k) === -1)
-  )
-
   return (
-    <Flow as={null} flowId={flowId} {...flowProps}>
+    <Flow
+      as={Dialog}
+      display="flex"
+      flexDirection="column"
+      flowId={flowId}
+      gap={5}
+      part={['announcement', part]}
+      textAlign="center"
+      {...props}
+    >
       {({
         flow,
         handleDismiss,
         handlePrimary,
         handleSecondary,
-        parentProps: { containerProps, dismissible },
+        parentProps: { dismissible },
         step,
       }) => {
-        const stepProps = step.props ?? {}
-
         const primaryButtonTitle = step.primaryButton?.title ?? step.primaryButtonTitle
         const secondaryButtonTitle = step.secondaryButton?.title ?? step.secondaryButtonTitle
 
         const disabled = step.$state.blocked
 
-        const { videoProps } = getVideoProps(stepProps)
+        const { videoProps } = getVideoProps(step.props ?? {})
 
         return (
-          <Dialog
-            part={['announcement', part]}
-            textAlign="center"
-            {...containerProps}
-            {...dialogProps}
-            onEscapeKeyDown={(e) => {
-              if (props.dismissible === false) {
-                e.preventDefault()
-                return
-              }
-              if (typeof props.onEscapeKeyDown === 'function') {
-                props.onEscapeKeyDown(e)
-              }
-
-              if (!e.defaultPrevented) {
-                handleDismiss(e)
-              }
-            }}
-          >
-            <Flex.Column gap={5} part="announcement-step" {...stepProps}>
-              {dismissible && <Dialog.Dismiss onClick={handleDismiss} />}
-              <Flex.Column gap={1} part="announcement-header">
-                <Dialog.Title>{step.title}</Dialog.Title>
-                <Dialog.Subtitle>{step.subtitle}</Dialog.Subtitle>
-              </Flex.Column>
+          <>
+            {dismissible && <Dialog.Dismiss onClick={handleDismiss} />}
+            <Flex.Column gap={1} part="announcement-header">
+              <Dialog.Title>{step.title}</Dialog.Title>
+              <Dialog.Subtitle>{step.subtitle}</Dialog.Subtitle>
+            </Flex.Column>
 
-              <Dialog.Media
-                aspectRatio="1.5"
-                objectFit="cover"
-                overflowClipMargin="unset"
-                src={step.videoUri ?? step.imageUri}
-                transform="translate3d(0, 0, 1px)"
-                type={step.videoUri ? 'video' : 'image'}
-                width="100%"
-                {...videoProps}
-              />
+            <Dialog.Media
+              aspectRatio="1.5"
+              objectFit="cover"
+              overflowClipMargin="unset"
+              src={step.videoUri ?? step.imageUri}
+              transform="translate3d(0, 0, 1px)"
+              type={step.videoUri ? 'video' : 'image'}
+              width="100%"
+              {...videoProps}
+            />
 
-              <Dialog.ProgressDots
-                current={flow.getCurrentStepIndex()}
-                total={flow.getNumberOfAvailableSteps()}
-              />
+            <Dialog.ProgressDots
+              current={flow.getCurrentStepIndex()}
+              total={flow.getNumberOfAvailableSteps()}
+            />
 
-              <Flex.Row
-                css={{
-                  '& > button': {
-                    flexBasis: '50%',
-                    flexGrow: 1,
-                  },
-                }}
-                gap={3}
-                part="announcement-footer"
-              >
-                {secondaryButtonTitle && (
-                  <Dialog.Secondary
-                    disabled={disabled}
-                    onClick={handleSecondary}
-                    title={secondaryButtonTitle}
-                  />
-                )}
-                {primaryButtonTitle && (
-                  <Dialog.Primary
-                    disabled={disabled}
-                    onClick={handlePrimary}
-                    title={primaryButtonTitle}
-                  />
-                )}
-              </Flex.Row>
-            </Flex.Column>
-          </Dialog>
+            <Flex.Row
+              css={{
+                '& > button': {
+                  flexBasis: '50%',
+                  flexGrow: 1,
+                },
+              }}
+              gap={3}
+              part="announcement-footer"
+            >
+              {secondaryButtonTitle && (
+                <Dialog.Secondary
+                  disabled={disabled}
+                  onClick={handleSecondary}
+                  title={secondaryButtonTitle}
+                />
+              )}
+              {primaryButtonTitle && (
+                <Dialog.Primary
+                  disabled={disabled}
+                  onClick={handlePrimary}
+                  title={primaryButtonTitle}
+                />
+              )}
+            </Flex.Row>
+          </>
         )
       }}
     </Flow>

packages/react/src/components/Banner/index.tsx

@@ -5,37 +5,30 @@ import { Box } from '@/components/Box'
 
 export interface BannerProps extends FlowPropsWithoutChildren {}
 
-export function Banner({ dismissible, flowId, part, ...props }: BannerProps) {
+export function Banner({ flowId, part, ...props }: BannerProps) {
   return (
-    <Flow as={null} flowId={flowId} {...props}>
-      {({
-        handleDismiss,
-        handlePrimary,
-        handleSecondary,
-        parentProps: { containerProps },
-        step,
-      }) => {
-        const stepProps = step.props ?? {}
-
+    <Flow
+      alignItems="center"
+      aria-label="Banner"
+      as={Card}
+      borderWidth="md"
+      display="flex"
+      flexDirection="row"
+      flowId={flowId}
+      gap={3}
+      justifyContent="flex-start"
+      part={['banner', part]}
+      role="complementary"
+      {...props}
+    >
+      {({ handleDismiss, handlePrimary, handleSecondary, parentProps: { dismissible }, step }) => {
         const primaryButtonTitle = step.primaryButton?.title ?? step.primaryButtonTitle
         const secondaryButtonTitle = step.secondaryButton?.title ?? step.secondaryButtonTitle
 
         const disabled = step.$state.blocked
 
         return (
-          <Card
-            alignItems="center"
-            aria-label="Banner"
-            borderWidth="md"
-            display="flex"
-            flexDirection="row"
-            gap={3}
-            justifyContent="flex-start"
-            part={['banner', part]}
-            role="complementary"
-            {...containerProps}
-            {...stepProps}
-          >
+          <>
             {step.imageUri && (
               <Box
                 as="img"
@@ -57,7 +50,7 @@ export function Banner({ dismissible, flowId, part, ...props }: BannerProps) {
             />
             <Card.Primary disabled={disabled} title={primaryButtonTitle} onClick={handlePrimary} />
             {dismissible && <Card.Dismiss onClick={handleDismiss} />}
-          </Card>
+          </>
         )
       }}
     </Flow>

packages/react/src/components/Card/index.tsx

@@ -86,6 +86,14 @@ Card.Footer = ({ children, part, ...props }: BoxProps) => {
 }
 
 Card.Header = ({ dismissible, handleDismiss, part, subtitle, title, ...props }) => {
+  if (
+    !dismissible &&
+    (title == null || title?.length === 0) &&
+    (subtitle == null || subtitle?.length === 0)
+  ) {
+    return null
+  }
+
   return (
     <Flex.Row
       alignItems="flex-start"

packages/react/src/components/Checklist/Collapsible.tsx

@@ -141,7 +141,6 @@ function StepWrapper({ flow, step, ...props }: FlowChildrenProps) {
 }
 
 export function Collapsible({
-  dismissible,
   flowId,
   onPrimary,
   onSecondary,
@@ -179,6 +178,9 @@ export function Collapsible({
           const currentSteps = flow.getNumberOfCompletedSteps()
           const availableSteps = flow.getNumberOfAvailableSteps()
 
+          // Note: Ignore merged props from step here, Checklist steps don't control flow dismissibility
+          const dismissible = props.dismissible || !!flow?.props?.dismissible
+
           return (
             <>
               <Flex.Column gap={2}>

packages/react/src/components/Flow/index.tsx

@@ -9,6 +9,7 @@ import { useStepHandlers } from '@/hooks/useStepHandlers'
 import { useCheckForModalCollision } from '@/hooks/useModal'
 
 import type { FlowProps } from '@/components/Flow/FlowProps'
+import { getVideoProps } from '@/components/Media/videoProps'
 // import { FrigadeContext } from '@/components/Provider'
 
 export type {
@@ -17,6 +18,10 @@ export type {
   FlowPropsWithoutChildren,
 } from '@/components/Flow/FlowProps'
 
+function isDialog(component) {
+  return typeof component === 'function' && component.displayName === 'Dialog'
+}
+
 export function Flow({
   as,
   children,
@@ -35,19 +40,25 @@ export function Flow({
     variables,
   })
 
+  const step = flow?.getCurrentStep()
+
+  const initialStepProps = step?.props ?? {}
+
+  // Discard video props when merging step props onto top-level container
+  const { otherProps: stepProps } = getVideoProps(initialStepProps)
+
   const {
-    dismissible = false,
+    dismissible = isDialog(as) ? true : false,
     forceMount = false,
     ...mergedProps
   } = {
     ...props,
     ...(flow?.props ?? {}),
+    ...(flow?.rawData?.flowType === FlowType.CHECKLIST ? {} : stepProps),
   }
 
   // const { hasInitialized, registerComponent, unregisterComponent } = useContext(FrigadeContext)
 
-  const step = flow?.getCurrentStep()
-
   const { handleDismiss } = useFlowHandlers(flow, {
     onComplete,
     onDismiss,
@@ -60,11 +71,26 @@ export function Flow({
 
   const isModal =
     mergedProps?.modal ||
-    (typeof as === 'function' && as?.displayName === 'Dialog') ||
+    isDialog(as) ||
     [FlowType.ANNOUNCEMENT, FlowType.TOUR].includes(flow?.rawData?.flowType)
 
   const { hasModalCollision } = useCheckForModalCollision(flow, isModal)
 
+  function handleEscapeKeyDown(e) {
+    if (dismissible === false) {
+      e.preventDefault()
+      return
+    }
+
+    if (typeof props.onEscapeKeyDown === 'function') {
+      props.onEscapeKeyDown(e)
+    }
+
+    if (!e.defaultPrevented) {
+      handleDismiss(e)
+    }
+  }
+
   // useEffect(() => {
   //   return () => {
   //     unregisterComponent(flowId)
@@ -103,11 +129,15 @@ export function Flow({
 
   const ContainerElement = as === null ? Fragment : as ?? Box
 
-  const containerProps = {
+  const containerProps: Record<string, unknown> = {
     ...mergedProps,
     'data-flow-id': flow.id,
   }
 
+  if (isDialog(as)) {
+    containerProps.onEscapeKeyDown = handleEscapeKeyDown
+  }
+
   return (
     <ContainerElement {...(as === null ? {} : containerProps)}>
       {children({