Browse Source

refactor some modal to fix early return problem

Yuki Takei 5 months ago
parent
commit
c571402791

+ 34 - 13
apps/app/src/client/components/GrantedGroupsInheritanceSelectModal.tsx

@@ -9,13 +9,20 @@ import {
   useGrantedGroupsInheritanceSelectModalActions, useGrantedGroupsInheritanceSelectModalStatus,
 } from '~/states/ui/modal/granted-groups-inheritance-select';
 
-const GrantedGroupsInheritanceSelectModal = (): React.JSX.Element => {
+/**
+ * GrantedGroupsInheritanceSelectModalSubstance - Presentation component (heavy logic, rendered only when isOpen)
+ */
+type GrantedGroupsInheritanceSelectModalSubstanceProps = {
+  onCreateBtnClick: ((onlyInheritUserRelatedGrantedGroups: boolean) => Promise<void>) | undefined;
+  closeModal: () => void;
+};
+
+const GrantedGroupsInheritanceSelectModalSubstance = (props: GrantedGroupsInheritanceSelectModalSubstanceProps): React.JSX.Element => {
+  const { onCreateBtnClick: _onCreateBtnClick, closeModal } = props;
   const { t } = useTranslation();
-  const { isOpened, onCreateBtnClick: _onCreateBtnClick } = useGrantedGroupsInheritanceSelectModalStatus();
-  const { close: closeModal } = useGrantedGroupsInheritanceSelectModalActions();
+
   const [onlyInheritUserRelatedGrantedGroups, setOnlyInheritUserRelatedGrantedGroups] = useState(false);
 
-  // Memoize event handlers
   const onCreateBtnClick = useCallback(async() => {
     await _onCreateBtnClick?.(onlyInheritUserRelatedGrantedGroups);
     setOnlyInheritUserRelatedGrantedGroups(false); // reset to false after create request
@@ -24,16 +31,8 @@ const GrantedGroupsInheritanceSelectModal = (): React.JSX.Element => {
   const setInheritAll = useCallback(() => setOnlyInheritUserRelatedGrantedGroups(false), []);
   const setInheritRelatedOnly = useCallback(() => setOnlyInheritUserRelatedGrantedGroups(true), []);
 
-  // Early return optimization
-  if (!isOpened) {
-    return <></>;
-  }
-
   return (
-    <Modal
-      isOpen={isOpened}
-      toggle={() => closeModal()}
-    >
+    <>
       <ModalHeader tag="h4" toggle={() => closeModal()}>
         {t('modal_granted_groups_inheritance_select.select_granted_groups')}
       </ModalHeader>
@@ -73,6 +72,28 @@ const GrantedGroupsInheritanceSelectModal = (): React.JSX.Element => {
           {t('modal_granted_groups_inheritance_select.create_page')}
         </button>
       </ModalFooter>
+    </>
+  );
+};
+
+/**
+ * GrantedGroupsInheritanceSelectModal - Container component (lightweight, always rendered)
+ */
+const GrantedGroupsInheritanceSelectModal = (): React.JSX.Element => {
+  const { isOpened, onCreateBtnClick } = useGrantedGroupsInheritanceSelectModalStatus();
+  const { close: closeModal } = useGrantedGroupsInheritanceSelectModalActions();
+
+  return (
+    <Modal
+      isOpen={isOpened}
+      toggle={() => closeModal()}
+    >
+      {isOpened && (
+        <GrantedGroupsInheritanceSelectModalSubstance
+          onCreateBtnClick={onCreateBtnClick}
+          closeModal={closeModal}
+        />
+      )}
     </Modal>
   );
 };

+ 25 - 13
apps/app/src/client/components/Me/AssociateModal.tsx

@@ -22,13 +22,20 @@ type Props = {
   onClose: () => void,
 }
 
-const AssociateModal = (props: Props): JSX.Element => {
+/**
+ * AssociateModalSubstance - Presentation component (heavy logic, rendered only when isOpen)
+ */
+type AssociateModalSubstanceProps = {
+  onClose: () => void;
+};
+
+const AssociateModalSubstance = (props: AssociateModalSubstanceProps): JSX.Element => {
+  const { onClose } = props;
   const { t } = useTranslation();
   const { mutate: mutatePersonalExternalAccounts } = useSWRxPersonalExternalAccounts();
-  const { trigger: associateLdapAccount, isMutating } = useAssociateLdapAccount();
-  const [activeTab, setActiveTab] = useState(1);
-  const { isOpen, onClose } = props;
+  const { trigger: associateLdapAccount } = useAssociateLdapAccount();
 
+  const [activeTab, setActiveTab] = useState(1);
   const [username, setUsername] = useState('');
   const [password, setPassword] = useState('');
 
@@ -38,7 +45,6 @@ const AssociateModal = (props: Props): JSX.Element => {
     setPassword('');
   }, [onClose]);
 
-
   const clickAddLdapAccountHandler = useCallback(async() => {
     try {
       await associateLdapAccount({ username, password });
@@ -50,23 +56,16 @@ const AssociateModal = (props: Props): JSX.Element => {
     catch (err) {
       toastError(err);
     }
-
   }, [associateLdapAccount, closeModalHandler, mutatePersonalExternalAccounts, password, t, username]);
 
-  // Memoize event handlers
   const setTabToLdap = useCallback(() => setActiveTab(1), []);
   const setTabToGithub = useCallback(() => setActiveTab(2), []);
   const setTabToGoogle = useCallback(() => setActiveTab(3), []);
   const handleUsernameChange = useCallback((username: string) => setUsername(username), []);
   const handlePasswordChange = useCallback((password: string) => setPassword(password), []);
 
-  // Early return optimization
-  if (!isOpen) {
-    return <></>;
-  }
-
   return (
-    <Modal isOpen={isOpen} toggle={closeModalHandler} size="lg" data-testid="grw-associate-modal">
+    <>
       <ModalHeader toggle={onClose}>
         { t('admin:user_management.create_external_account') }
       </ModalHeader>
@@ -122,6 +121,19 @@ const AssociateModal = (props: Props): JSX.Element => {
           {t('add')}
         </button>
       </ModalFooter>
+    </>
+  );
+};
+
+/**
+ * AssociateModal - Container component (lightweight, always rendered)
+ */
+const AssociateModal = (props: Props): JSX.Element => {
+  const { isOpen, onClose } = props;
+
+  return (
+    <Modal isOpen={isOpen} toggle={onClose} size="lg" data-testid="grw-associate-modal">
+      {isOpen && <AssociateModalSubstance onClose={onClose} />}
     </Modal>
   );
 };

+ 28 - 10
apps/app/src/client/components/Me/DisassociateModal.tsx

@@ -19,10 +19,16 @@ type Props = {
   accountForDisassociate: IExternalAccount<IExternalAuthProviderType> & HasObjectId,
 }
 
+/**
+ * DisassociateModalSubstance - Presentation component (heavy logic, rendered only when isOpen)
+ */
+type DisassociateModalSubstanceProps = {
+  onClose: () => void;
+  accountForDisassociate: IExternalAccount<IExternalAuthProviderType> & HasObjectId;
+};
 
-const DisassociateModal = (props: Props): React.JSX.Element => {
-  const { isOpen, onClose, accountForDisassociate } = props;
-
+const DisassociateModalSubstance = (props: DisassociateModalSubstanceProps): React.JSX.Element => {
+  const { onClose, accountForDisassociate } = props;
   const { t } = useTranslation();
   const { mutate: mutatePersonalExternalAccounts } = useSWRxPersonalExternalAccounts();
   const { trigger: disassociateLdapAccount } = useDisassociateLdapAccount();
@@ -30,7 +36,6 @@ const DisassociateModal = (props: Props): React.JSX.Element => {
   const { providerType, accountId } = accountForDisassociate;
 
   const disassociateAccountHandler = useCallback(async() => {
-
     try {
       await disassociateLdapAccount({ providerType, accountId });
       onClose();
@@ -45,13 +50,8 @@ const DisassociateModal = (props: Props): React.JSX.Element => {
     }
   }, [accountId, disassociateLdapAccount, mutatePersonalExternalAccounts, onClose, providerType, t]);
 
-  // Early return optimization
-  if (!isOpen) {
-    return <></>;
-  }
-
   return (
-    <Modal isOpen={isOpen} toggle={onClose}>
+    <>
       <ModalHeader className="text-info" toggle={onClose}>
         {t('personal_settings.disassociate_external_account')}
       </ModalHeader>
@@ -68,6 +68,24 @@ const DisassociateModal = (props: Props): React.JSX.Element => {
           { t('Disassociate') }
         </button>
       </ModalFooter>
+    </>
+  );
+};
+
+/**
+ * DisassociateModal - Container component (lightweight, always rendered)
+ */
+const DisassociateModal = (props: Props): React.JSX.Element => {
+  const { isOpen, onClose, accountForDisassociate } = props;
+
+  return (
+    <Modal isOpen={isOpen} toggle={onClose}>
+      {isOpen && (
+        <DisassociateModalSubstance
+          onClose={onClose}
+          accountForDisassociate={accountForDisassociate}
+        />
+      )}
     </Modal>
   );
 };