From ecff9fac4d9bc228595abe224a1828e9c5315ba1 Mon Sep 17 00:00:00 2001 From: Antonin Cezard Date: Tue, 29 Nov 2022 09:57:55 +0100 Subject: [PATCH] fix: handle `open` prop in Dialog component --- react/Dialog/DialogEffects.spec.tsx | 244 ++++++++++++++++++ react/Dialog/DialogEffects.ts | 63 ++++- react/Dialog/index.jsx | 2 +- .../useSetFlagshipUi/useSetFlagshipUI.ts | 2 +- 4 files changed, 305 insertions(+), 6 deletions(-) diff --git a/react/Dialog/DialogEffects.spec.tsx b/react/Dialog/DialogEffects.spec.tsx index b4f5feadf1..89d0ab6c36 100644 --- a/react/Dialog/DialogEffects.spec.tsx +++ b/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 => ({}) +})) + +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() + const service = { + call: (...args: unknown[]): void => caller(...args) + } as WebviewService + + const { unmount } = render( + + + + + + ) + + 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() + const service = { + call: (...args: unknown[]): void => caller(...args) + } as WebviewService + + const { rerender } = render( + + + + + + ) + + expect(caller).toHaveBeenNthCalledWith( + 1, + 'setFlagshipUI', + onOpenMountExpected, + 'cozy-ui/Dialog (onOpenMount)' + ) + + rerender( + + ) + + 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() + const service = { + call: (...args: unknown[]): void => caller(...args) + } as WebviewService + + const { rerender, unmount } = render( + + + + + + ) + + expect(caller).not.toHaveBeenCalled() + + rerender( + + + + + + ) + + expect(caller).toHaveBeenNthCalledWith( + 1, + 'setFlagshipUI', + onOpenMountExpected, + 'cozy-ui/Dialog (onOpenMount)' + ) + + rerender( + + + + + + ) + + expect(caller).toHaveBeenNthCalledWith( + 2, + 'setFlagshipUI', + onOpenUnmountExpected, + 'cozy-ui/Dialog (onOpenUnmount)' + ) + + rerender( + + + + + + ) + + 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 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() + const service = { + call: (...args: unknown[]): void => caller(...args) + } as WebviewService + + const { unmount } = render( + + + + + + ) + + 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 that has no open prop, and then fixed at runtime, it should emit mount and unmount messages as shown earlier', () => { + const caller = jest.fn() + const service = { + call: (...args: unknown[]): void => caller(...args) + } as WebviewService + + const { rerender, unmount } = render( + + + + + + ) + + expect(caller).not.toHaveBeenCalled() + + rerender( + + + + + + ) + + expect(caller).toHaveBeenNthCalledWith( + 1, + 'setFlagshipUI', + onOpenMountExpected, + 'cozy-ui/Dialog (onOpenMount)' + ) + + rerender( + + + + + + ) + + 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) +}) diff --git a/react/Dialog/DialogEffects.ts b/react/Dialog/DialogEffects.ts index a30205fc62..e88c40fb2a 100644 --- a/react/Dialog/DialogEffects.ts +++ b/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 diff --git a/react/Dialog/index.jsx b/react/Dialog/index.jsx index 635e23eab9..8e9ae2b7b8 100644 --- a/react/Dialog/index.jsx +++ b/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 ( diff --git a/react/hooks/useSetFlagshipUi/useSetFlagshipUI.ts b/react/hooks/useSetFlagshipUi/useSetFlagshipUI.ts index 8278853023..7ea9436eae 100644 --- a/react/hooks/useSetFlagshipUi/useSetFlagshipUI.ts +++ b/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