2
0
Эх сурвалжийг харах

refactor: replace useCheckboxState with useCheckbox for improved checkbox management and integrate with tree features

Yuki Takei 4 сар өмнө
parent
commit
df2c0d034a

+ 15 - 24
apps/app/src/features/page-tree/components/ItemsTree.tsx

@@ -1,5 +1,5 @@
 import type { FC } from 'react';
-import { useCallback, useMemo } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
 import { useTree } from '@headless-tree/react';
 import { useVirtualizer } from '@tanstack/react-virtual';
 import { useTranslation } from 'next-i18next';
@@ -11,8 +11,6 @@ import { useSWRxRootPage } from '~/stores/page-listing';
 import { ROOT_PAGE_VIRTUAL_ID } from '../constants/_inner';
 import {
   useAutoExpandAncestors,
-  useCheckboxChangeNotification,
-  useCheckboxState,
   useDataLoader,
   useExpandParentOnCreate,
   useScrollToSelectedItem,
@@ -20,7 +18,6 @@ import {
   useTreeItemHandlers,
   useTreeRevalidation,
 } from '../hooks/_inner';
-import { usePageDnd } from '../hooks/use-page-dnd';
 import { useSocketUpdateDescCount } from '../hooks/use-socket-update-desc-count';
 import type { TreeItemProps } from '../interfaces';
 import { useTriggerTreeRebuild } from '../states/_inner';
@@ -84,18 +81,16 @@ export const ItemsTree: FC<Props> = (props: Props) => {
   const { getItemName, isItemFolder, handleRename, creatingParentId } =
     useTreeItemHandlers(triggerTreeRebuild);
 
-  // Configure tree features based on options
-  const features = useTreeFeatures({
+  // Configure tree features and get checkbox state and D&D handlers
+  const { features, checkboxProperties, dndProperties } = useTreeFeatures({
     enableRenaming,
     enableCheckboxes,
     enableDragAndDrop,
+    initialCheckedItems,
   });
 
-  // Page move (drag and drop) handlers
-  const { canDrag, canDrop, onDrop, renderDragLine } = usePageDnd(enableDragAndDrop);
-
-  // Subscribe to Socket.io UpdateDescCount events
-  useSocketUpdateDescCount();
+  const { setCheckedItems, createNotifyEffect } = checkboxProperties;
+  const { canDrag, canDrop, onDrop, renderDragLine } = dndProperties;
 
   // Wrap onDrop to show toast notifications
   const handleDrop = useCallback(
@@ -112,12 +107,6 @@ export const ItemsTree: FC<Props> = (props: Props) => {
     [onDrop, t],
   );
 
-  // Manage checkbox state (must be called before useTree to get setCheckedItems)
-  const { checkedItemIds, setCheckedItems } = useCheckboxState({
-    enabled: enableCheckboxes,
-    initialCheckedItems,
-  });
-
   // Stable initial state
   // biome-ignore lint/correctness/useExhaustiveDependencies: initialCheckedItems is intentionally not in deps to avoid reinitializing on every change
   const initialState = useMemo(
@@ -146,17 +135,19 @@ export const ItemsTree: FC<Props> = (props: Props) => {
       canDrag,
       canDrop,
       onDrop: handleDrop,
-      canDropInbetween: false, // No reordering, only drop as child
+      canDropInbetween: false,
     }),
   });
 
   // Notify parent when checked items change
-  useCheckboxChangeNotification({
-    enabled: enableCheckboxes,
-    checkedItemIds,
+  // biome-ignore lint/correctness/useExhaustiveDependencies: createNotifyEffect already includes checkedItemIds in its closure
+  useEffect(createNotifyEffect(tree, onCheckedItemsChange), [
+    createNotifyEffect,
     tree,
-    onCheckedItemsChange,
-  });
+  ]);
+
+  // Subscribe to Socket.io UpdateDescCount events
+  useSocketUpdateDescCount();
 
   // Handle tree revalidation and items count tracking
   useTreeRevalidation({ tree, triggerTreeRebuild });
@@ -232,7 +223,7 @@ export const ItemsTree: FC<Props> = (props: Props) => {
           </div>
         );
       })}
-      {/* Drag line indicator (rendered by usePageDnd when D&D is enabled) */}
+      {/* Drag line indicator (rendered by dndProperties when D&D is enabled) */}
       {renderDragLine(tree)}
     </div>
   );

+ 1 - 1
apps/app/src/features/page-tree/hooks/_inner/index.ts

@@ -1,5 +1,5 @@
 export * from './use-auto-expand-ancestors';
-export * from './use-checkbox-state';
+export * from './use-checkbox';
 export * from './use-data-loader';
 export * from './use-expand-parent-on-create';
 export * from './use-scroll-to-selected-item';

+ 0 - 70
apps/app/src/features/page-tree/hooks/_inner/use-checkbox-state.ts

@@ -1,70 +0,0 @@
-import { useCallback, useEffect, useState } from 'react';
-
-import type { IPageForTreeItem } from '~/interfaces/page';
-
-type TreeInstance = {
-  getItemInstance: (
-    id: string,
-  ) => { getItemData: () => IPageForTreeItem } | undefined;
-};
-
-export type UseCheckboxStateOptions = {
-  enabled: boolean;
-  initialCheckedItems: string[];
-};
-
-export type UseCheckboxStateReturn = {
-  checkedItemIds: string[];
-  setCheckedItems: ((itemIds: string[]) => void) | undefined;
-};
-
-/**
- * Hook to manage checkbox state for headless-tree.
- * Provides state tracking and setter callback for checked items.
- */
-export const useCheckboxState = (
-  options: UseCheckboxStateOptions,
-): UseCheckboxStateReturn => {
-  const { enabled, initialCheckedItems } = options;
-
-  // State to track checked items for re-rendering
-  const [checkedItemIds, setCheckedItemIds] =
-    useState<string[]>(initialCheckedItems);
-
-  // Callback to update checked items state (triggers re-render)
-  const handleSetCheckedItems = useCallback((itemIds: string[]) => {
-    setCheckedItemIds(itemIds);
-  }, []);
-
-  return {
-    checkedItemIds,
-    setCheckedItems: enabled ? handleSetCheckedItems : undefined,
-  };
-};
-
-export type UseCheckboxChangeNotificationOptions = {
-  enabled: boolean;
-  checkedItemIds: string[];
-  tree: TreeInstance;
-  onCheckedItemsChange?: (checkedItems: IPageForTreeItem[]) => void;
-};
-
-/**
- * Hook to notify parent when checked items change.
- */
-export const useCheckboxChangeNotification = (
-  options: UseCheckboxChangeNotificationOptions,
-): void => {
-  const { enabled, checkedItemIds, tree, onCheckedItemsChange } = options;
-
-  useEffect(() => {
-    if (!enabled || onCheckedItemsChange == null) {
-      return;
-    }
-
-    const checkedPages = checkedItemIds
-      .map((id) => tree.getItemInstance(id)?.getItemData())
-      .filter((page): page is IPageForTreeItem => page != null);
-    onCheckedItemsChange(checkedPages);
-  }, [enabled, checkedItemIds, onCheckedItemsChange, tree]);
-};

+ 81 - 0
apps/app/src/features/page-tree/hooks/_inner/use-checkbox.ts

@@ -0,0 +1,81 @@
+import { useCallback, useMemo, useState } from 'react';
+
+import type { IPageForTreeItem } from '~/interfaces/page';
+
+type TreeInstance = {
+  getItemInstance: (
+    id: string,
+  ) => { getItemData: () => IPageForTreeItem } | undefined;
+};
+
+export type UseCheckboxOptions = {
+  enabled: boolean;
+  initialCheckedItems: string[];
+};
+
+export type UseCheckboxProperties = {
+  checkedItemIds: string[];
+  setCheckedItems: ((itemIds: string[]) => void) | undefined;
+  /**
+   * Helper to create a useEffect callback for notifying parent of checked items changes.
+   * Usage in component:
+   * ```
+   * useEffect(
+   *   checkboxProperties.createNotifyEffect(tree, onCheckedItemsChange),
+   *   [checkboxProperties.checkedItemIds, tree, onCheckedItemsChange]
+   * );
+   * ```
+   */
+  createNotifyEffect: (
+    tree: TreeInstance,
+    onCheckedItemsChange?: (checkedItems: IPageForTreeItem[]) => void,
+  ) => () => void;
+};
+
+/**
+ * Hook to manage checkbox state for headless-tree.
+ * Provides state tracking and setter callback for checked items.
+ */
+export const useCheckbox = (
+  options: UseCheckboxOptions,
+): UseCheckboxProperties => {
+  const { enabled, initialCheckedItems } = options;
+
+  // State to track checked items for re-rendering
+  const [checkedItemIds, setCheckedItemIds] =
+    useState<string[]>(initialCheckedItems);
+
+  // Callback to update checked items state (triggers re-render)
+  const handleSetCheckedItems = useCallback((itemIds: string[]) => {
+    setCheckedItemIds(itemIds);
+  }, []);
+
+  // Helper to create useEffect callback for notifying parent
+  const createNotifyEffect = useCallback(
+    (
+      tree: TreeInstance,
+      onCheckedItemsChange?: (checkedItems: IPageForTreeItem[]) => void,
+    ) => {
+      return () => {
+        if (!enabled || onCheckedItemsChange == null) {
+          return;
+        }
+
+        const checkedPages = checkedItemIds
+          .map((id) => tree.getItemInstance(id)?.getItemData())
+          .filter((page): page is IPageForTreeItem => page != null);
+        onCheckedItemsChange(checkedPages);
+      };
+    },
+    [enabled, checkedItemIds],
+  );
+
+  return useMemo(
+    () => ({
+      checkedItemIds,
+      setCheckedItems: enabled ? handleSetCheckedItems : undefined,
+      createNotifyEffect,
+    }),
+    [checkedItemIds, enabled, handleSetCheckedItems, createNotifyEffect],
+  );
+};

+ 40 - 8
apps/app/src/features/page-tree/hooks/_inner/use-tree-features.ts

@@ -9,44 +9,76 @@ import {
   selectionFeature,
 } from '@headless-tree/core';
 
+import type { UsePageDndProperties } from '../use-page-dnd';
+import { usePageDnd } from '../use-page-dnd';
+import type { UseCheckboxProperties } from './use-checkbox';
+import { useCheckbox } from './use-checkbox';
+
 export type UseTreeFeaturesOptions = {
   enableRenaming?: boolean;
   enableCheckboxes?: boolean;
   enableDragAndDrop?: boolean;
+  initialCheckedItems?: string[];
+};
+
+export type UseTreeFeaturesResult = {
+  features: FeatureImplementation<unknown>[];
+  checkboxProperties: UseCheckboxProperties;
+  dndProperties: UsePageDndProperties;
 };
 
 /**
  * Hook to configure tree features based on options.
- * Returns a stable array of features for use with headless-tree.
+ * Returns a stable array of features for use with headless-tree,
+ * along with checkbox state and page D&D handlers.
  */
 export const useTreeFeatures = (
   options: UseTreeFeaturesOptions = {},
-): FeatureImplementation<unknown>[] => {
+): UseTreeFeaturesResult => {
   const {
     enableRenaming = true,
     enableCheckboxes = false,
     enableDragAndDrop = false,
+    initialCheckedItems = [],
   } = options;
 
-  return useMemo(() => {
-    const features: FeatureImplementation<unknown>[] = [
+  // Get checkbox properties
+  const checkboxProperties = useCheckbox({
+    enabled: enableCheckboxes,
+    initialCheckedItems,
+  });
+
+  // Get page D&D handlers
+  const dndProperties = usePageDnd(enableDragAndDrop);
+
+  const features = useMemo(() => {
+    const featureList: FeatureImplementation<unknown>[] = [
       asyncDataLoaderFeature,
       selectionFeature,
       hotkeysCoreFeature,
     ];
 
     if (enableRenaming) {
-      features.push(renamingFeature);
+      featureList.push(renamingFeature);
     }
 
     if (enableCheckboxes) {
-      features.push(checkboxesFeature);
+      featureList.push(checkboxesFeature);
     }
 
     if (enableDragAndDrop) {
-      features.push(dragAndDropFeature);
+      featureList.push(dragAndDropFeature);
     }
 
-    return features;
+    return featureList;
   }, [enableRenaming, enableCheckboxes, enableDragAndDrop]);
+
+  return useMemo(
+    () => ({
+      features,
+      checkboxProperties,
+      dndProperties,
+    }),
+    [features, checkboxProperties, dndProperties],
+  );
 };

+ 2 - 2
apps/app/src/features/page-tree/hooks/use-page-dnd.tsx

@@ -86,7 +86,7 @@ const DragLine: FC<DragLineProps> = ({ style, className }) => (
   />
 );
 
-export type UsePageDndResult = {
+export type UsePageDndProperties = {
   canDrag: (items: ItemInstance<IPageForTreeItem>[]) => boolean;
   canDrop: (
     items: ItemInstance<IPageForTreeItem>[],
@@ -117,7 +117,7 @@ export type UsePageDndResult = {
  *
  * @returns Object with canDrag, canDrop, onDrop handlers and renderDragLine
  */
-export const usePageDnd = (isEnabled: boolean = false): UsePageDndResult => {
+export const usePageDnd = (isEnabled: boolean = false): UsePageDndProperties => {
   const { notifyUpdateItems } = usePageTreeInformationUpdate();
 
   /**