fix: handle open prop in Dialog component

react/Dialog/DialogEffects.spec.tsx

@@ -1,5 +1,12 @@
+import '@testing-library/jest-dom'
+import React from 'react'
 import { Theme } from '@material-ui/core'
+import { render } from '@testing-library/react'
 
+import { WebviewIntentProvider, WebviewService } from 'cozy-intent'
+
+import Dialog from '.'
+import { BreakpointsProvider } from '../hooks/useBreakpoints'
 import { DOMStrings, makeOnMount, makeOnUnmount } from './DialogEffects'
 import { ThemeColor } from '../hooks/useSetFlagshipUi/useSetFlagshipUI'
 
@@ -219,3 +226,240 @@ it('should provide the inversed UI when Cozybar is not black on white, but white
     topTheme: ThemeColor.Light
   })
 })
+
+jest.mock('cozy-device-helper', () => ({
+  isFlagshipApp: (): boolean => true,
+  getFlagshipMetadata: (): Record<string, never> => ({})
+}))
+
+const onOpenMountExpected = {
+  bottomBackground: '#fff',
+  bottomOverlay: 'rgba(0, 0, 0, 0.5)',
+  bottomTheme: 'light',
+  topOverlay: 'rgba(0, 0, 0, 0.5)',
+  topTheme: 'light'
+}
+
+const onOpenUnmountExpected = {
+  bottomBackground: '#fff',
+  bottomOverlay: 'transparent',
+  bottomTheme: 'dark',
+  topOverlay: 'transparent',
+  topTheme: 'dark'
+}
+
+it('should emit onMount() immediately and onUnmount() when the whole tree is deleted', () => {
+  const caller = jest.fn<void, unknown[]>()
+  const service = {
+    call: (...args: unknown[]): void => caller(...args)
+  } as WebviewService
+
+  const { unmount } = render(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    1,
+    'setFlagshipUI',
+    onOpenMountExpected,
+    'cozy-ui/Dialog (onOpenMount)'
+  )
+
+  unmount()
+
+  expect(caller).toHaveBeenNthCalledWith(
+    2,
+    'setFlagshipUI',
+    onOpenUnmountExpected,
+    'cozy-ui/Dialog (onOpenUnmount)'
+  )
+})
+
+it('should emit onMount() immediately and onUnmount() when Dialog is deleted from the tree', () => {
+  const caller = jest.fn<void, unknown[]>()
+  const service = {
+    call: (...args: unknown[]): void => caller(...args)
+  } as WebviewService
+
+  const { rerender } = render(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    1,
+    'setFlagshipUI',
+    onOpenMountExpected,
+    'cozy-ui/Dialog (onOpenMount)'
+  )
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}></WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    2,
+    'setFlagshipUI',
+    onOpenUnmountExpected,
+    'cozy-ui/Dialog (onOpenUnmount)'
+  )
+})
+
+it('should not emit onMount() if mounted as open:false, then emit onMount() on open:true, then emit onUnmount() on switch back to open:false, then emit onMount() again on switch back to open:true, then emit onUnmount() again when the tree is deleted', () => {
+  const caller = jest.fn<void, unknown[]>()
+  const service = {
+    call: (...args: unknown[]): void => caller(...args)
+  } as WebviewService
+
+  const { rerender, unmount } = render(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open={false} />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).not.toHaveBeenCalled()
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open={true} />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    1,
+    'setFlagshipUI',
+    onOpenMountExpected,
+    'cozy-ui/Dialog (onOpenMount)'
+  )
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open={false} />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    2,
+    'setFlagshipUI',
+    onOpenUnmountExpected,
+    'cozy-ui/Dialog (onOpenUnmount)'
+  )
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open={true} />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    3,
+    'setFlagshipUI',
+    onOpenMountExpected,
+    'cozy-ui/Dialog (onOpenMount)'
+  )
+
+  unmount()
+
+  expect(caller).toHaveBeenNthCalledWith(
+    4,
+    'setFlagshipUI',
+    onOpenUnmountExpected,
+    'cozy-ui/Dialog (onOpenUnmount)'
+  )
+})
+
+it('when provided with a faulty <Dialog /> that has no open prop, it should emit nothing at mount or unmount', () => {
+  /** Using Dialog without open prop is forbidden, so we acknowledge that but since we need to test it we silence the console */
+  jest.spyOn(console, 'error').mockImplementation(() => {
+    // do nothing
+  })
+
+  const caller = jest.fn<void, unknown[]>()
+  const service = {
+    call: (...args: unknown[]): void => caller(...args)
+  } as WebviewService
+
+  const { unmount } = render(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).not.toHaveBeenCalled()
+
+  unmount()
+
+  /** As it was mounted without the open prop, and we never changed it, then it never sent an onMount message.
+   * In this scenario, it is only logical that it does not send an unMount message either since it never showed up.
+   */
+  expect(caller).not.toHaveBeenCalled()
+})
+
+it('when provided with a faulty <Dialog /> that has no open prop, and then fixed at runtime, it should emit mount and unmount messages as shown earlier', () => {
+  const caller = jest.fn<void, unknown[]>()
+  const service = {
+    call: (...args: unknown[]): void => caller(...args)
+  } as WebviewService
+
+  const { rerender, unmount } = render(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).not.toHaveBeenCalled()
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    1,
+    'setFlagshipUI',
+    onOpenMountExpected,
+    'cozy-ui/Dialog (onOpenMount)'
+  )
+
+  rerender(
+    <WebviewIntentProvider webviewService={service}>
+      <BreakpointsProvider>
+        <Dialog open={false} />
+      </BreakpointsProvider>
+    </WebviewIntentProvider>
+  )
+
+  expect(caller).toHaveBeenNthCalledWith(
+    2,
+    'setFlagshipUI',
+    onOpenUnmountExpected,
+    'cozy-ui/Dialog (onOpenUnmount)'
+  )
+
+  unmount()
+
+  /** Checking it doesn't send two unmount messages in a row */
+  expect(caller).toHaveBeenCalledTimes(2)
+})

react/Dialog/DialogEffects.ts

@@ -1,11 +1,13 @@
 import { getLuminance, Theme, useTheme } from '@material-ui/core'
+import { useEffect } from 'react'
 
 import { getFlagshipMetadata, isFlagshipApp } from 'cozy-device-helper'
+import { useWebviewIntent } from 'cozy-intent'
 
 import {
   FlagshipUI,
   ThemeColor,
-  useSetFlagshipUI
+  parseArg
 } from '../hooks/useSetFlagshipUi/useSetFlagshipUI'
 
 interface DialogEffectsOptions {
@@ -120,23 +122,76 @@ const makeCaller = (
 const getRootModal = (): HTMLElement | null => {
   const modals = document.querySelectorAll(DOMStrings.DialogClass)
 
-  return modals.length > 0 ? (modals[0] as HTMLElement) : null
+  /**
+   * If we have more than one modal, we are in a stacked dialog scenario.
+   * In this case we want to have access to the DOM element of the root modal.
+   * This will allow us to apply the correct background color if a root modal exists, for instance.
+   */
+  return modals.length > 1 ? (modals[0] as HTMLElement) : null
 }
 
-const useHook = (fullscreen?: boolean): void => {
+const useHook = (open: boolean, fullscreen?: boolean): void => {
   const theme = useTheme()
   const cozybar = document.querySelector(DOMStrings.CozyBarClass)
   const sidebar = document.getElementById(DOMStrings.SidebarID)
   const rootModal = getRootModal()
   const immersive = Boolean(getFlagshipMetadata().immersive)
 
-  useSetFlagshipUI(
+  useDialogSetFlagshipUI(
+    open,
     makeOnMount({ fullscreen, theme, sidebar, rootModal, cozybar }),
     makeOnUnmount({ rootModal, theme, immersive, sidebar, cozybar }),
     makeCaller(!!fullscreen, !!rootModal, immersive)
   )
 }
 
+/**
+ * Custom version of useSetFlagshipUi() that is aware of the Dialog component.
+ *
+ * The difference here is that we send messages to the Native app when a props change.
+ * In the original version, we send the mount message as soon as the component is rendered.
+ *
+ * Dialog can be rendered but hidden, so we need to wait for the open prop to be true
+ */
+export const useDialogSetFlagshipUI = (
+  open: boolean,
+  onMount: FlagshipUI,
+  onUnmount?: FlagshipUI,
+  caller?: string
+): void => {
+  const webviewIntent = useWebviewIntent()
+
+  useEffect(() => {
+    if (open)
+      parseArg(webviewIntent, onMount, `${caller || 'unknown'} (onOpenMount)`)
+
+    return () => {
+      /**
+       * As we are listening to the open prop, we still want to send an unmount message when the prop changes to false.
+       * To avoid false positives, we need to ensure the component is currently showing.
+       * We do that by checking if value of open during this cleanup cycle is false,
+       * if it is, that means the component is currently showing and is in the process of hiding.
+       *
+       * Note that this will also handle abrupt unmounting, as in hiding the dialog without using the open prop.
+       */
+      if (open === false || open === undefined) return
+
+      parseArg(
+        webviewIntent,
+        onUnmount,
+        `${caller || 'unknown'} (onOpenUnmount)`
+      )
+    }
+
+    /**
+     * We don't want to listen to onMount/onUnmount arguments
+     * It will create far too many unwanted calls
+     * We only care about webviewIntent or open props presence,
+     * Open should always be present, webviewIntent is more uncertain
+     */
+  }, [open, webviewIntent]) // eslint-disable-line react-hooks/exhaustive-deps
+}
+
 export const useDialogEffects = isFlagshipApp()
   ? useHook
   : // eslint-disable-next-line @typescript-eslint/no-empty-function

react/Dialog/index.jsx

@@ -25,7 +25,7 @@ const Dialog = props => {
       : React.Fragment
   const cozyTheme = useCozyTheme()
 
-  useDialogEffects(props.fullScreen)
+  useDialogEffects(props.open, props.fullScreen)
 
   return (
     <Wrapper>

react/hooks/useSetFlagshipUi/useSetFlagshipUI.ts

@@ -18,7 +18,7 @@ export interface FlagshipUI
   bottomTheme: ThemeColor
 }
 
-const parseArg = (
+export const parseArg = (
   webviewIntent?: WebviewService | void,
   arg?: FlagshipUI,
   caller?: string