From d1d339e76e698954265c00e75677d75eaf4330ee Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 9 Dec 2022 13:39:51 +0100 Subject: [PATCH 1/4] fix(TypeScript): include `data-*` attributes in `CommonProps` interface --- packages/main/src/interfaces/CommonProps.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/main/src/interfaces/CommonProps.ts b/packages/main/src/interfaces/CommonProps.ts index fcd69874023..a078a39f1ad 100644 --- a/packages/main/src/interfaces/CommonProps.ts +++ b/packages/main/src/interfaces/CommonProps.ts @@ -1,6 +1,9 @@ import { CSSProperties, HTMLAttributes } from 'react'; -export interface CommonProps extends HTMLAttributes { +type DataAttributeKey = `data-${string}`; +type HTMLDataAttributes = Record; + +export interface CommonProps extends HTMLAttributes, HTMLDataAttributes { /** * Element style which will be appended to the most outer element of a component. * Use this prop carefully, some css properties might break the component. From 7cf882076ddc6a587f61d3b6d1019be123df969c Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 9 Dec 2022 13:41:23 +0100 Subject: [PATCH 2/4] fix(Modals - TypeScript): allow typing the container element --- .../src/components/Modals/ModalsProvider.tsx | 5 +- packages/main/src/components/Modals/index.tsx | 411 ++++++++++-------- packages/main/src/internal/ModalsContext.ts | 19 +- 3 files changed, 250 insertions(+), 185 deletions(-) diff --git a/packages/main/src/components/Modals/ModalsProvider.tsx b/packages/main/src/components/Modals/ModalsProvider.tsx index e4115889451..f382ef33bbc 100644 --- a/packages/main/src/components/Modals/ModalsProvider.tsx +++ b/packages/main/src/components/Modals/ModalsProvider.tsx @@ -6,10 +6,11 @@ export interface ModalsProviderPropTypes { children: ReactNode; } +//@ts-expect-error: can't assume state generics at this point const modalStateReducer = (state: ModalState[], action: UpdateModalStateAction) => { switch (action.type) { case 'set': - return [...state, action.payload as ModalState]; + return [...state, action.payload]; case 'reset': return state.filter((modal) => modal.id !== action.payload.id); default: @@ -34,7 +35,7 @@ export function ModalsProvider({ children }: ModalsProviderPropTypes) { {modals.map((modal) => { if (modal?.Component) { return createPortal( - , + , modal.container ?? document.body ); } diff --git a/packages/main/src/components/Modals/index.tsx b/packages/main/src/components/Modals/index.tsx index d25fb19a938..f4223b8b806 100644 --- a/packages/main/src/components/Modals/index.tsx +++ b/packages/main/src/components/Modals/index.tsx @@ -25,10 +25,13 @@ type ClosableModalReturnType = ModalReturnType & { close: () => void; }; -type ModalHookReturnType = (props: Props, container?: HTMLElement) => ModalReturnType; -type CloseableModalHookReturnType = ( +type ModalHookReturnType = ( props: Props, - container?: HTMLElement + container?: ContainerElement +) => ModalReturnType; +type CloseableModalHookReturnType = ( + props: Props, + container?: ContainerElement ) => ClosableModalReturnType; const checkContext = (context: any): void => { @@ -38,7 +41,11 @@ const checkContext = (context: any): void => { } }; -const showDialog = (props: any, setModal: React.Dispatch, container?: HTMLElement) => { +function showDialog( + props: DialogPropTypes, + setModal: React.Dispatch>, + container?: ContainerElement +) { checkContext(setModal); const id = getRandomId(); const ref = createRef(); @@ -48,7 +55,6 @@ const showDialog = (props: any, setModal: React.Dispatch Component: Dialog, props: { ...props, - ref, open: true, onAfterClose: (event) => { if (typeof props.onAfterClose === 'function') { @@ -60,14 +66,19 @@ const showDialog = (props: any, setModal: React.Dispatch }); } }, + ref, container, id } }); return { ref }; -}; +} -const showPopover = (props: any, setModal: React.Dispatch, container?: HTMLElement) => { +function showPopover( + props: PopoverPropTypes, + setModal: React.Dispatch>, + container?: ContainerElement +) { checkContext(setModal); const id = getRandomId(); const ref = createRef(); @@ -77,7 +88,7 @@ const showPopover = (props: any, setModal: React.Dispatch { if (typeof props.onAfterClose === 'function') { @@ -89,18 +100,21 @@ const showPopover = (props: any, setModal: React.Dispatch, - container?: HTMLElement -) => { +function showResponsivePopover( + props: ResponsivePopoverPropTypes, + setModal: React.Dispatch< + UpdateModalStateAction + >, + container?: ContainerElement +) { checkContext(setModal); const id = getRandomId(); const ref = createRef(); @@ -110,7 +124,6 @@ const showResponsivePopover = ( Component: ResponsivePopover, props: { ...props, - ref, open: true, onAfterClose: (event) => { if (typeof props.onAfterClose === 'function') { @@ -122,24 +135,29 @@ const showResponsivePopover = ( }); } }, + ref, container, id } }); return { ref }; -}; +} -const showMessageBox = (props: any, setModal: React.Dispatch, container?: HTMLElement) => { +function showMessageBox( + props: MessageBoxPropTypes, + setModal: React.Dispatch>, + container?: ContainerElement +) { checkContext(setModal); const id = getRandomId(); const ref = createRef(); setModal?.({ type: 'set', payload: { + // @ts-expect-error: `children` is required, but type-safety is provided through `props` Component: MessageBox, props: { ...props, - ref, open: true, onClose: (event) => { if (typeof props.onClose === 'function') { @@ -151,14 +169,19 @@ const showMessageBox = (props: any, setModal: React.Dispatch, container?: HTMLElement) => { +function showToast( + props: ToastPropTypes, + setModal: React.Dispatch>, + container?: ContainerElement +) { const ref = createRef() as MutableRefObject; checkContext(setModal); const id = getRandomId(); @@ -167,18 +190,18 @@ const showToast = (props: any, setModal: React.Dispatch, payload: { Component: Toast, props: { - ...props, - ref: (el: ToastDomRef) => { - ref.current = el; - if (el && !(el as any).open) { - el.show(); - setTimeout(() => { - setModal({ - type: 'reset', - payload: { id } - }); - }, props.duration ?? Toast.defaultProps.duration); - } + ...props + }, + ref: (el: ToastDomRef & { open: boolean }) => { + ref.current = el; + if (el && !el.open) { + el.show(); + setTimeout(() => { + setModal({ + type: 'reset', + payload: { id } + }); + }, props.duration ?? Toast.defaultProps.duration); } }, container, @@ -186,7 +209,178 @@ const showToast = (props: any, setModal: React.Dispatch, } }); return { ref }; -}; +} + +function showDialogFn( + props: DialogPropTypes, + container?: ContainerElement +): ClosableModalReturnType { + const setModal = window['@ui5/webcomponents-react']?.setModal; + + const { ref } = showDialog(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; +} + +function useShowDialogHook(): CloseableModalHookReturnType< + DialogPropTypes, + DialogDomRef, + ContainerElement +> { + const { setModal } = useModalsContext(); + + return useCallback( + (props, container) => { + const { ref } = showDialog(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; + }, + [setModal] + ); +} + +function showPopoverFn( + props: PopoverPropTypes, + container?: ContainerElement +): ClosableModalReturnType { + const setModal = window['@ui5/webcomponents-react']?.setModal; + const { ref } = showPopover(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; +} + +function useShowPopoverHook(): CloseableModalHookReturnType< + PopoverPropTypes, + PopoverDomRef, + ContainerElement +> { + const { setModal } = useModalsContext(); + return useCallback( + (props, container) => { + const { ref } = showPopover(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; + }, + [setModal] + ); +} + +function showResponsivePopoverFn( + props: ResponsivePopoverPropTypes, + container?: ContainerElement +): ClosableModalReturnType { + const setModal = window['@ui5/webcomponents-react']?.setModal; + const { ref } = showResponsivePopover(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; +} + +function useResponsivePopoverHook(): CloseableModalHookReturnType< + ResponsivePopoverPropTypes, + ResponsivePopoverDomRef, + ContainerElement +> { + const { setModal } = useModalsContext(); + return useCallback( + (props, container) => { + const { ref } = showResponsivePopover(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; + }, + [setModal] + ); +} + +function showMessageBoxFn( + props: MessageBoxPropTypes, + container?: ContainerElement +): ClosableModalReturnType { + const setModal = window['@ui5/webcomponents-react']?.setModal; + const { ref } = showMessageBox(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; +} +function useShowMessageBox(): CloseableModalHookReturnType< + MessageBoxPropTypes, + DialogDomRef, + ContainerElement +> { + const { setModal } = useModalsContext(); + return useCallback( + (props, container) => { + const { ref } = showMessageBox(props, setModal, container); + + return { + ref, + close: () => { + ref.current?.close(); + } + }; + }, + [setModal] + ); +} + +function showToastFn( + props: ToastPropTypes, + container?: ContainerElement +): ModalReturnType { + const setModal = window['@ui5/webcomponents-react']?.setModal; + const { ref } = showToast(props, setModal, container); + + return { + ref + }; +} + +function useShowToastHook(): ModalHookReturnType { + const { setModal } = useModalsContext(); + + return useCallback( + (props: ToastPropTypes, container?) => { + const { ref } = showToast(props, setModal, container); + return { + ref + }; + }, + [setModal] + ); +} /** * Utility class for opening modals in an imperative way. @@ -197,145 +391,14 @@ const showToast = (props: any, setModal: React.Dispatch, * @since 0.22.2 */ export const Modals = { - showDialog: (props: DialogPropTypes, container?: HTMLElement): ClosableModalReturnType => { - const setModal = window['@ui5/webcomponents-react']?.setModal; - const { ref } = showDialog(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - - useShowDialog: (): CloseableModalHookReturnType => { - const { setModal } = useModalsContext(); - - return useCallback( - (props, container) => { - const { ref } = showDialog(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - [setModal] - ); - }, - - showPopover: (props: PopoverPropTypes, container?: HTMLElement): ClosableModalReturnType => { - const setModal = window['@ui5/webcomponents-react']?.setModal; - const { ref } = showPopover(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - - useShowPopover: (): CloseableModalHookReturnType => { - const { setModal } = useModalsContext(); - return useCallback( - (props, container) => { - const { ref } = showPopover(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - [setModal] - ); - }, - - showResponsivePopover: ( - props: ResponsivePopoverPropTypes, - container?: HTMLElement - ): ClosableModalReturnType => { - const setModal = window['@ui5/webcomponents-react']?.setModal; - const { ref } = showResponsivePopover(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - - useShowResponsivePopover: (): CloseableModalHookReturnType => { - const { setModal } = useModalsContext(); - return useCallback( - (props, container) => { - const { ref } = showResponsivePopover(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - [setModal] - ); - }, - - showMessageBox: (props: MessageBoxPropTypes, container?: HTMLElement): ClosableModalReturnType => { - const setModal = window['@ui5/webcomponents-react']?.setModal; - const { ref } = showMessageBox(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - - useShowMessageBox: (): CloseableModalHookReturnType => { - const { setModal } = useModalsContext(); - return useCallback( - (props, container) => { - const { ref } = showMessageBox(props, setModal, container); - - return { - ref, - close: () => { - ref.current?.close(); - } - }; - }, - [setModal] - ); - }, - - showToast: (props: ToastPropTypes, container?: HTMLElement): ModalReturnType => { - const setModal = window['@ui5/webcomponents-react']?.setModal; - const { ref } = showToast(props, setModal, container); - - return { - ref - }; - }, - useShowToast: (): ModalHookReturnType => { - const { setModal } = useModalsContext(); - - return useCallback( - (props: ToastPropTypes, container?: HTMLElement) => { - const { ref } = showToast(props, setModal, container); - return { - ref - }; - }, - [setModal] - ); - } + showDialog: showDialogFn, + useShowDialog: useShowDialogHook, + showPopover: showPopoverFn, + useShowPopover: useShowPopoverHook, + showResponsivePopover: showResponsivePopoverFn, + useShowResponsivePopover: useResponsivePopoverHook, + showMessageBox: showMessageBoxFn, + useShowMessageBox: useShowMessageBox, + showToast: showToastFn, + useShowToast: useShowToastHook }; diff --git a/packages/main/src/internal/ModalsContext.ts b/packages/main/src/internal/ModalsContext.ts index 6af07be7be5..38b315a3680 100644 --- a/packages/main/src/internal/ModalsContext.ts +++ b/packages/main/src/internal/ModalsContext.ts @@ -1,22 +1,23 @@ -import { ComponentType, ContextType, createContext, Dispatch, useContext } from 'react'; +import { ComponentType, ContextType, createContext, Dispatch, RefCallback, RefObject, useContext } from 'react'; -export interface UpdateModalStateAction { +export interface UpdateModalStateAction { type: 'set' | 'reset'; - payload?: ModalState | { id: string }; + payload?: ModalState | { id: string }; } -export interface ModalState { +export interface ModalState { Component: ComponentType; - props: Record; - container: HTMLElement; + props: Props; + ref: RefObject | RefCallback; + container: ContainerElement; id: string; } -interface IModalsContext { - setModal?: Dispatch; +interface IModalsContext { + setModal?: Dispatch>; } -export const ModalsContext = createContext({ +export const ModalsContext = createContext, HTMLElement>>({ setModal: null }); From 2babb4bf2870b97ee5e68ea500ac49fef97c757e Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 9 Dec 2022 15:34:55 +0100 Subject: [PATCH 3/4] remove data attributes from CommonProps --- packages/main/src/interfaces/CommonProps.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/main/src/interfaces/CommonProps.ts b/packages/main/src/interfaces/CommonProps.ts index a078a39f1ad..fcd69874023 100644 --- a/packages/main/src/interfaces/CommonProps.ts +++ b/packages/main/src/interfaces/CommonProps.ts @@ -1,9 +1,6 @@ import { CSSProperties, HTMLAttributes } from 'react'; -type DataAttributeKey = `data-${string}`; -type HTMLDataAttributes = Record; - -export interface CommonProps extends HTMLAttributes, HTMLDataAttributes { +export interface CommonProps extends HTMLAttributes { /** * Element style which will be appended to the most outer element of a component. * Use this prop carefully, some css properties might break the component. From 5d0b4681557719a8874dd6b68658f1ebecbd59ee Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 9 Dec 2022 15:37:02 +0100 Subject: [PATCH 4/4] fix tests --- .../src/components/Modals/Modals.test.tsx | 41 +++++++++++-------- packages/main/src/components/Modals/index.tsx | 1 - 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/main/src/components/Modals/Modals.test.tsx b/packages/main/src/components/Modals/Modals.test.tsx index faddaeaac35..c6a148bbf08 100644 --- a/packages/main/src/components/Modals/Modals.test.tsx +++ b/packages/main/src/components/Modals/Modals.test.tsx @@ -1,15 +1,17 @@ import { act, fireEvent, render, screen } from '@shared/tests'; import React from 'react'; +import { DialogPropTypes, MessageBoxPropTypes, PopoverPropTypes, ResponsivePopoverPropTypes } from '../..'; import { Modals } from './index'; describe('Modals - static helpers', function () { test('showDialog', () => { render(null); + const props: DialogPropTypes & { 'data-testid': string } = { + children: 'Dialog Content', + 'data-testid': 'dialog' + }; act(() => { - Modals.showDialog({ - children: 'Dialog Content', - 'data-testid': 'dialog' - }); + Modals.showDialog(props); }); expect(screen.getByTestId('dialog')).toHaveAttribute('open', 'true'); @@ -18,12 +20,13 @@ describe('Modals - static helpers', function () { test('showPopover', () => { render(); + const props: PopoverPropTypes & { 'data-testid': string } = { + opener: 'opener', + children: 'Popover Content', + 'data-testid': 'popover' + }; act(() => { - Modals.showPopover({ - opener: 'opener', - children: 'Popover Content', - 'data-testid': 'popover' - }); + Modals.showPopover(props); }); expect(screen.getByTestId('popover')).toHaveAttribute('open', 'true'); @@ -32,12 +35,13 @@ describe('Modals - static helpers', function () { test('showResponsivePopover', () => { render(); + const props: ResponsivePopoverPropTypes & { 'data-testid': string } = { + opener: 'opener', + children: 'ResponsivePopover Content', + 'data-testid': 'responsivepopover' + }; act(() => { - Modals.showResponsivePopover({ - opener: 'opener', - children: 'ResponsivePopover Content', - 'data-testid': 'responsivepopover' - }); + Modals.showResponsivePopover(props); }); expect(screen.getByTestId('responsivepopover')).toHaveAttribute('open', 'true'); @@ -46,11 +50,12 @@ describe('Modals - static helpers', function () { test('showMessageBox', async () => { render(null); + const props: MessageBoxPropTypes & { 'data-testid': string } = { + children: 'MessageBox Content', + 'data-testid': 'messagebox' + }; act(() => { - Modals.showMessageBox({ - children: 'MessageBox Content', - 'data-testid': 'messagebox' - }); + Modals.showMessageBox(props); }); await screen.findByText('Confirmation'); diff --git a/packages/main/src/components/Modals/index.tsx b/packages/main/src/components/Modals/index.tsx index f4223b8b806..84a5049b4fe 100644 --- a/packages/main/src/components/Modals/index.tsx +++ b/packages/main/src/components/Modals/index.tsx @@ -154,7 +154,6 @@ function showMessageBox( setModal?.({ type: 'set', payload: { - // @ts-expect-error: `children` is required, but type-safety is provided through `props` Component: MessageBox, props: { ...props,