Modal API Refactoring (#12062)
# Modal API Refactoring This PR refactors the modal system to use an imperative approach for setting hotkey scopes, addressing race conditions that occurred with the previous effect-based implementation. Fixes #11986 Closes #12087 ## Key Changes: - **New Modal API**: Introduced a `useModal` hook with `openModal`, `closeModal`, and `toggleModal` functions, similar to the existing dropdown API - **Modal Identification**: Added a `modalId` prop to uniquely identify modals - **State Management**: Introduced `isModalOpenedComponentState` and removed individual boolean state atoms (like `isRemoveSortingModalOpenState`) - **Modal Constants**: Added consistent modal ID constants (e.g., `FavoriteFolderDeleteModalId`, `RecordIndexRemoveSortingModalId`) for better maintainability - **Mount Effects**: Created mount effect components (like `AuthModalMountEffect`) to handle initial modal opening where needed ## Implementation Details: - Modified `Modal` and `ConfirmationModal` components to accept the new `modalId` prop - Added a component-state-based approach using `ModalComponentInstanceContext` to track modal state - Introduced imperative modal handlers that properly manage hotkey scopes - Components like `ActionModal` and `AttachmentList` now use the new `useModal` hook for better control over modal state ## Benefits: - **Race Condition Prevention**: Hotkey scopes are now set imperatively, eliminating race conditions - **Consistent API**: Modal and dropdown now share similar patterns, improving developer experience ## Tests to do before merging: 1. Action Modals (Modal triggered by an action, for example the delete action) 2. Auth Modal (`AuthModal.tsx` and `AuthModalMountEffect.tsx`) - Test that auth modal opens automatically on mount - Verify authentication flow works properly 3. Email Verification Sent Modal (in `SignInUp.tsx`) - Verify this modal displays correctly 4. Attachment Preview Modal (in `AttachmentList.tsx`) - Test opening preview modal by clicking on attachments - Verify close, download functionality works - Test modal navigation and interactions 5. Favorite Folder Delete Modal (`CurrentWorkspaceMemberFavorites.tsx`) - Test deletion confirmation flow - Check that modal opens when attempting to delete folders with favorites 6. Record Board Remove Sorting Modal (`RecordBoard.tsx` using `RecordIndexRemoveSortingModalId`) - Test that modal appears when trying to drag records with sorting enabled - Verify sorting removal works correctly 7. Record Group Reorder Confirmation Modal (`RecordGroupReorderConfirmationModal.tsx`) - Test group reordering with sorting enabled - Verify confirmation modal properly handles sorting removal 8. Confirmation Modal (base component used by several modals) - Test all variants with different confirmation options For each modal, verify: - Opening/closing behavior - Hotkey support (Esc to close, Enter to confirm where applicable) - Click outside behavior - Proper z-index stacking - Any modal-specific functionality
This commit is contained in:
Raphaël Bosi
@ -1,18 +1,16 @@
|
||||
import { RootStackingContextZIndices } from '@/ui/layout/constants/RootStackingContextZIndices';
|
||||
import { ModalHotkeysAndClickOutsideEffect } from '@/ui/layout/modal/components/ModalHotkeysAndClickOutsideEffect';
|
||||
import { ModalHotkeyScope } from '@/ui/layout/modal/components/types/ModalHotkeyScope';
|
||||
import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope';
|
||||
import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys';
|
||||
import {
|
||||
ClickOutsideMode,
|
||||
useListenClickOutside,
|
||||
} from '@/ui/utilities/pointer-event/hooks/useListenClickOutside';
|
||||
import { ModalComponentInstanceContext } from '@/ui/layout/modal/contexts/ModalComponentInstanceContext';
|
||||
import { isModalOpenedComponentState } from '@/ui/layout/modal/states/isModalOpenedComponentState';
|
||||
|
||||
import { useModal } from '@/ui/layout/modal/hooks/useModal';
|
||||
import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile';
|
||||
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
|
||||
import { css, useTheme } from '@emotion/react';
|
||||
import styled from '@emotion/styled';
|
||||
import { motion } from 'framer-motion';
|
||||
import React, { useEffect, useRef } from 'react';
|
||||
import { Key } from 'ts-key-enum';
|
||||
|
||||
import React, { useRef } from 'react';
|
||||
const StyledModalDiv = styled(motion.div)<{
|
||||
size?: ModalSize;
|
||||
padding?: ModalPadding;
|
||||
@ -169,6 +167,7 @@ export type ModalPadding = 'none' | 'small' | 'medium' | 'large';
|
||||
export type ModalVariants = 'primary' | 'secondary' | 'tertiary';
|
||||
|
||||
export type ModalProps = React.PropsWithChildren & {
|
||||
modalId: string;
|
||||
size?: ModalSize;
|
||||
padding?: ModalPadding;
|
||||
className?: string;
|
||||
@ -176,7 +175,7 @@ export type ModalProps = React.PropsWithChildren & {
|
||||
onEnter?: () => void;
|
||||
modalVariant?: ModalVariants;
|
||||
} & (
|
||||
| { isClosable: true; onClose: () => void }
|
||||
| { isClosable: true; onClose?: () => void }
|
||||
| { isClosable?: false; onClose?: never }
|
||||
);
|
||||
|
||||
@ -187,11 +186,11 @@ const modalAnimation = {
|
||||
};
|
||||
|
||||
export const Modal = ({
|
||||
modalId,
|
||||
children,
|
||||
size = 'medium',
|
||||
padding = 'medium',
|
||||
className,
|
||||
hotkeyScope = ModalHotkeyScope.Default,
|
||||
onEnter,
|
||||
isClosable = false,
|
||||
onClose,
|
||||
@ -200,80 +199,65 @@ export const Modal = ({
|
||||
const isMobile = useIsMobile();
|
||||
const modalRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
const {
|
||||
goBackToPreviousHotkeyScope,
|
||||
setHotkeyScopeAndMemorizePreviousScope,
|
||||
} = usePreviousHotkeyScope();
|
||||
|
||||
useEffect(() => {
|
||||
setHotkeyScopeAndMemorizePreviousScope(hotkeyScope);
|
||||
return () => {
|
||||
goBackToPreviousHotkeyScope();
|
||||
};
|
||||
}, [
|
||||
hotkeyScope,
|
||||
setHotkeyScopeAndMemorizePreviousScope,
|
||||
goBackToPreviousHotkeyScope,
|
||||
]);
|
||||
|
||||
useScopedHotkeys(
|
||||
[Key.Enter],
|
||||
() => {
|
||||
onEnter?.();
|
||||
},
|
||||
hotkeyScope,
|
||||
);
|
||||
|
||||
useScopedHotkeys(
|
||||
[Key.Escape],
|
||||
() => {
|
||||
if (isClosable && onClose !== undefined) {
|
||||
onClose();
|
||||
}
|
||||
},
|
||||
hotkeyScope,
|
||||
);
|
||||
|
||||
useListenClickOutside({
|
||||
refs: [modalRef],
|
||||
listenerId: 'MODAL_CLICK_OUTSIDE_LISTENER_ID',
|
||||
callback: () => {
|
||||
if (isClosable && onClose !== undefined) {
|
||||
onClose();
|
||||
}
|
||||
},
|
||||
mode: ClickOutsideMode.comparePixels,
|
||||
});
|
||||
const theme = useTheme();
|
||||
|
||||
const stopEventPropagation = (e: React.MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
};
|
||||
|
||||
const theme = useTheme();
|
||||
const isModalOpened = useRecoilComponentValueV2(
|
||||
isModalOpenedComponentState,
|
||||
modalId,
|
||||
);
|
||||
|
||||
const { closeModal } = useModal();
|
||||
|
||||
const handleClose = () => {
|
||||
onClose?.();
|
||||
closeModal(modalId);
|
||||
};
|
||||
|
||||
return (
|
||||
<StyledBackDrop
|
||||
className="modal-backdrop"
|
||||
onMouseDown={stopEventPropagation}
|
||||
modalVariant={modalVariant}
|
||||
>
|
||||
<StyledModalDiv
|
||||
ref={modalRef}
|
||||
size={size}
|
||||
padding={padding}
|
||||
initial="hidden"
|
||||
animate="visible"
|
||||
exit="exit"
|
||||
layout
|
||||
modalVariant={modalVariant}
|
||||
variants={modalAnimation}
|
||||
transition={{ duration: theme.animation.duration.normal }}
|
||||
className={className}
|
||||
isMobile={isMobile}
|
||||
>
|
||||
{children}
|
||||
</StyledModalDiv>
|
||||
</StyledBackDrop>
|
||||
<>
|
||||
{isModalOpened && (
|
||||
<ModalComponentInstanceContext.Provider
|
||||
value={{
|
||||
instanceId: modalId,
|
||||
}}
|
||||
>
|
||||
<ModalHotkeysAndClickOutsideEffect
|
||||
modalId={modalId}
|
||||
modalRef={modalRef}
|
||||
onEnter={onEnter}
|
||||
isClosable={isClosable}
|
||||
onClose={handleClose}
|
||||
/>
|
||||
<StyledBackDrop
|
||||
data-testid="modal-backdrop"
|
||||
className="modal-backdrop"
|
||||
onMouseDown={stopEventPropagation}
|
||||
modalVariant={modalVariant}
|
||||
>
|
||||
<StyledModalDiv
|
||||
ref={modalRef}
|
||||
size={size}
|
||||
padding={padding}
|
||||
initial="hidden"
|
||||
animate="visible"
|
||||
exit="exit"
|
||||
layout
|
||||
modalVariant={modalVariant}
|
||||
variants={modalAnimation}
|
||||
transition={{ duration: theme.animation.duration.normal }}
|
||||
className={className}
|
||||
isMobile={isMobile}
|
||||
>
|
||||
{children}
|
||||
</StyledModalDiv>
|
||||
</StyledBackDrop>
|
||||
</ModalComponentInstanceContext.Provider>
|
||||
)}
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user