Merge pull request #9073 from weseek/fix/supress-rendering-invisible-dropdown-menus

fix: Supress rendering too many invisible DropdownMenu components

apps/app/src/client/components/Bookmarks/BookmarkFolderItemControl.tsx

@@ -26,37 +26,40 @@ export const BookmarkFolderItemControl: React.FC<{
           <span className="material-symbols-outlined">more_horiz</span>
         </DropdownToggle>
       ) }
-      <DropdownMenu
-        container="body"
-        style={{ zIndex: 1055 }} /* make it larger than $zindex-modal of bootstrap */
-      >
-        {onClickMoveToRoot && (
+
+      { isOpen && (
+        <DropdownMenu
+          container="body"
+          style={{ zIndex: 1055 }}
+        >
+          {onClickMoveToRoot && (
+            <DropdownItem
+              onClick={onClickMoveToRoot}
+              className="grw-page-control-dropdown-item"
+            >
+              <span className="material-symbols-outlined grw-page-control-dropdown-icon">bookmark</span>
+              {t('bookmark_folder.move_to_root')}
+            </DropdownItem>
+          )}
           <DropdownItem
-            onClick={onClickMoveToRoot}
+            onClick={onClickRename}
             className="grw-page-control-dropdown-item"
           >
-            <span className="material-symbols-outlined grw-page-control-dropdown-icon">bookmark</span>
-            {t('bookmark_folder.move_to_root')}
+            <span className="material-symbols-outlined me-1 grw-page-control-dropdown-icon">redo</span>
+            {t('Rename')}
           </DropdownItem>
-        )}
-        <DropdownItem
-          onClick={onClickRename}
-          className="grw-page-control-dropdown-item"
-        >
-          <span className="material-symbols-outlined me-1 grw-page-control-dropdown-icon">redo</span>
-          {t('Rename')}
-        </DropdownItem>
 
-        <DropdownItem divider />
+          <DropdownItem divider />
 
-        <DropdownItem
-          className="pt-2 grw-page-control-dropdown-item text-danger"
-          onClick={onClickDelete}
-        >
-          <span className="material-symbols-outlined me-1 grw-page-control-dropdown-icon">delete</span>
-          {t('Delete')}
-        </DropdownItem>
-      </DropdownMenu>
+          <DropdownItem
+            className="pt-2 grw-page-control-dropdown-item text-danger"
+            onClick={onClickDelete}
+          >
+            <span className="material-symbols-outlined me-1 grw-page-control-dropdown-icon">delete</span>
+            {t('Delete')}
+          </DropdownItem>
+        </DropdownMenu>
+      ) }
     </Dropdown>
   );
 };

apps/app/src/client/components/Bookmarks/BookmarkFolderMenu.tsx

@@ -186,15 +186,18 @@ export const BookmarkFolderMenu = (props: BookmarkFolderMenuProps): JSX.Element
       onToggle={toggleHandler}
     >
       {children}
-      <DropdownMenu
-        end
-        persist
-        strategy="fixed"
-        container="body"
-        className={`grw-bookmark-folder-menu ${styles['grw-bookmark-folder-menu']}`}
-      >
-        { renderBookmarkMenuItem() }
-      </DropdownMenu>
+
+      { isOpen && (
+        <DropdownMenu
+          end
+          persist
+          strategy="fixed"
+          container="body"
+          className={`grw-bookmark-folder-menu ${styles['grw-bookmark-folder-menu']}`}
+        >
+          { renderBookmarkMenuItem() }
+        </DropdownMenu>
+      ) }
     </UncontrolledDropdown>
   );
 };

apps/app/src/client/components/Common/Dropdown/PageItemControl.spec.tsx

@@ -1,37 +1,54 @@
-import { render, screen, waitFor } from '@testing-library/react';
-import userEvent, { PointerEventsCheckLevel } from '@testing-library/user-event';
+import { type IPageInfoForOperation } from '@growi/core/dist/interfaces';
+import {
+  fireEvent, render, screen, within,
+} from '@testing-library/react';
+import { mock } from 'vitest-mock-extended';
 
 import { PageItemControl } from './PageItemControl';
 
 
+// mock for isIPageInfoForOperation
+
+const mocks = vi.hoisted(() => ({
+  isIPageInfoForOperationMock: vi.fn(),
+}));
+
+vi.mock('@growi/core/dist/interfaces', () => ({
+  isIPageInfoForOperation: mocks.isIPageInfoForOperationMock,
+}));
+
+
 describe('PageItemControl.tsx', () => {
-  it('Should trigger onClickRenameMenuItem() when clicking the rename button with pageInfo.isDeletable being "false"', async() => {
-    // setup
-    const onClickRenameMenuItemMock = vi.fn();
-
-    const pageInfo = {
-      isMovable: true,
-      isV5Compatible: true,
-      isEmpty: false,
-      isDeletable: false,
-      isAbleToDeleteCompletely: true,
-      isRevertible: true,
-    };
-
-    const props = {
-      pageId: 'dummy-page-id',
-      isEnableActions: true,
-      pageInfo,
-      onClickRenameMenuItem: onClickRenameMenuItemMock,
-    };
-
-    render(<PageItemControl {...props} />);
-
-    // when
-    const openPageMoveRenameModalButton = screen.getByTestId('rename-page-btn');
-    await waitFor(() => userEvent.click(openPageMoveRenameModalButton, { pointerEventsCheck: PointerEventsCheckLevel.Never }));
-
-    // then
-    expect(onClickRenameMenuItemMock).toHaveBeenCalled();
+  describe('Should trigger onClickRenameMenuItem() when clicking the rename button', () => {
+    it('without fetching PageInfo by useSWRxPageInfo', async() => {
+      // setup
+      const pageInfo = mock<IPageInfoForOperation>();
+
+      const onClickRenameMenuItemMock = vi.fn();
+      // return true when the argument is pageInfo in order to supress fetching
+      mocks.isIPageInfoForOperationMock.mockImplementation((arg) => {
+        if (arg === pageInfo) {
+          return true;
+        }
+      });
+
+      const props = {
+        pageId: 'dummy-page-id',
+        isEnableActions: true,
+        pageInfo,
+        onClickRenameMenuItem: onClickRenameMenuItemMock,
+      };
+
+      render(<PageItemControl {...props} />);
+
+      // when
+      const button = within(screen.getByTestId('open-page-item-control-btn')).getByText(/more_vert/);
+      fireEvent.click(button);
+      const renameMenuItem = await screen.findByTestId('rename-page-btn');
+      fireEvent.click(renameMenuItem);
+
+      // then
+      expect(onClickRenameMenuItemMock).toHaveBeenCalled();
+    });
   });
 });

apps/app/src/client/components/Common/Dropdown/PageItemControl.tsx

@@ -4,7 +4,7 @@ import React, {
 
 import {
   type IPageInfoAll, isIPageInfoForOperation,
-} from '@growi/core';
+} from '@growi/core/dist/interfaces';
 import { LoadingSpinner } from '@growi/ui/dist/components';
 import { useTranslation } from 'next-i18next';
 import {
@@ -338,21 +338,23 @@ export const PageItemControlSubstance = (props: PageItemControlSubstanceProps):
     <NotAvailableForGuest>
       <Dropdown isOpen={isOpen} toggle={() => setIsOpen(!isOpen)} className="grw-page-item-control" data-testid="open-page-item-control-btn">
         { children ?? (
-          <DropdownToggle color="transparent" className="border-0 rounded btn-page-item-control d-flex align-items-center justify-content-center">
+          <DropdownToggle role="button" color="transparent" className="border-0 rounded btn-page-item-control d-flex align-items-center justify-content-center">
             <span className="material-symbols-outlined">more_vert</span>
           </DropdownToggle>
         ) }
 
-        <PageItemControlDropdownMenu
-          {...props}
-          isLoading={isLoading}
-          pageInfo={fetchedPageInfo ?? presetPageInfo}
-          onClickBookmarkMenuItem={bookmarkMenuItemClickHandler}
-          onClickRenameMenuItem={renameMenuItemClickHandler}
-          onClickDuplicateMenuItem={duplicateMenuItemClickHandler}
-          onClickDeleteMenuItem={deleteMenuItemClickHandler}
-          onClickPathRecoveryMenuItem={pathRecoveryMenuItemClickHandler}
-        />
+        { isOpen && (
+          <PageItemControlDropdownMenu
+            {...props}
+            isLoading={isLoading}
+            pageInfo={fetchedPageInfo ?? presetPageInfo}
+            onClickBookmarkMenuItem={bookmarkMenuItemClickHandler}
+            onClickRenameMenuItem={renameMenuItemClickHandler}
+            onClickDuplicateMenuItem={duplicateMenuItemClickHandler}
+            onClickDeleteMenuItem={deleteMenuItemClickHandler}
+            onClickPathRecoveryMenuItem={pathRecoveryMenuItemClickHandler}
+          />
+        ) }
       </Dropdown>
 
     </NotAvailableForGuest>

apps/app/src/client/components/InAppNotification/InAppNotificationDropdown.tsx

@@ -86,18 +86,21 @@ export const InAppNotificationDropdown = (): JSX.Element => {
       <DropdownToggle className="px-3" color="primary" innerRef={buttonRef}>
         <span className="material-symbols-outlined">notifications</span> {badge}
       </DropdownToggle>
-      <DropdownMenu end>
-        { inAppNotificationData != null && inAppNotificationData.docs.length === 0
+
+      { isOpen && (
+        <DropdownMenu end>
+          { inAppNotificationData != null && inAppNotificationData.docs.length === 0
           // no items
-          ? <DropdownItem disabled>{t('in_app_notification.mark_all_as_read')}</DropdownItem>
+            ? <DropdownItem disabled>{t('in_app_notification.mark_all_as_read')}</DropdownItem>
           // render DropdownItem
-          : <InAppNotificationList inAppNotificationData={inAppNotificationData} />
-        }
-        <DropdownItem divider />
-        <DropdownItem tag="a" href="/me/all-in-app-notifications">
-          { t('in_app_notification.see_all') }
-        </DropdownItem>
-      </DropdownMenu>
+            : <InAppNotificationList inAppNotificationData={inAppNotificationData} />
+          }
+          <DropdownItem divider />
+          <DropdownItem tag="a" href="/me/all-in-app-notifications">
+            { t('in_app_notification.see_all') }
+          </DropdownItem>
+        </DropdownMenu>
+      ) }
     </Dropdown>
   );
 };