Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: PageTree auto-scrolling sometimges not woking #9544

Merged
merged 8 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/app/public/static/locales/en_US/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,8 @@
"pagetree": {
"cannot_rename_a_title_that_contains_slash": "Cannot rename a title that contains '/'",
"you_cannot_move_this_page_now": "You cannot move this page now",
"something_went_wrong_with_moving_page": "Something went wrong with moving page"
"something_went_wrong_with_moving_page": "Something went wrong with moving page",
"error_retrieving_the_pagetree": "Error occurred while retrieving the PageTree"
},
"duplicated_page_alert": {
"same_page_name_exists": "Same page name exits as「{{pageName}}」",
Expand Down
3 changes: 2 additions & 1 deletion apps/app/public/static/locales/fr_FR/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,8 @@
"pagetree": {
"cannot_rename_a_title_that_contains_slash": "Renommage impossible lorsque le titre contient '/'",
"you_cannot_move_this_page_now": "Déplacement de la page impossible",
"something_went_wrong_with_moving_page": "Échec de déplacement de la page"
"something_went_wrong_with_moving_page": "Échec de déplacement de la page",
"error_retrieving_the_pagetree": "Une erreur s'est produite lors de la récupération de l'arbre des pages"
},
"duplicated_page_alert": {
"same_page_name_exists": "Une page avec ce nom 「{{pageName}}」 existe déjà",
Expand Down
3 changes: 2 additions & 1 deletion apps/app/public/static/locales/ja_JP/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,8 @@
"pagetree": {
"cannot_rename_a_title_that_contains_slash": "`/` が含まれているタイトルにリネームできません",
"you_cannot_move_this_page_now": "現在、このページを移動することはできません",
"something_went_wrong_with_moving_page": "ページの移動に問題が発生しました"
"something_went_wrong_with_moving_page": "ページの移動に問題が発生しました",
"error_retrieving_the_pagetree": "ページツリーの取得中にエラーが発生しました"
},
"duplicated_page_alert": {
"same_page_name_exists": "ページ名 「{{pageName}}」が重複しています",
Expand Down
3 changes: 2 additions & 1 deletion apps/app/public/static/locales/zh_CN/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,8 @@
"pagetree": {
"cannot_rename_a_title_that_contains_slash": "不能重命名包含 ’/' 的标题",
"you_cannot_move_this_page_now": "你现在不能移动这个页面",
"something_went_wrong_with_moving_page": "移动页面时出了问题"
"something_went_wrong_with_moving_page": "移动页面时出了问题",
"error_retrieving_the_pagetree": "检索页面树时发生错误"
},
"duplicated_page_alert": {
"same_page_name_exists": "页面名称「{{pageName}}」是重复的",
Expand Down
100 changes: 10 additions & 90 deletions apps/app/src/client/components/ItemsTree/ItemsTree.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
import React, {
useEffect, useMemo, useCallback,
useEffect, useCallback,
} from 'react';

import path from 'path';

import type { Nullable, IPageHasId, IPageToDeleteWithMeta } from '@growi/core';
import type { IPageToDeleteWithMeta } from '@growi/core';
import { useGlobalSocket } from '@growi/core/dist/swr';
import { useTranslation } from 'next-i18next';
import { useRouter } from 'next/router';

import { toastError, toastSuccess } from '~/client/util/toastr';
import type { IPageForItem } from '~/interfaces/page';
import type { AncestorsChildrenResult, RootPageResult, TargetAndAncestors } from '~/interfaces/page-listing-results';
import type { OnDuplicatedFunction, OnDeletedFunction } from '~/interfaces/ui';
import type { UpdateDescCountData, UpdateDescCountRawData } from '~/interfaces/websocket';
import { SocketEventName } from '~/interfaces/websocket';
import type { IPageForPageDuplicateModal } from '~/stores/modal';
import { usePageDuplicateModal, usePageDeleteModal } from '~/stores/modal';
import { mutateAllPageInfo, useCurrentPagePath, useSWRMUTxCurrentPage } from '~/stores/page';
import {
useSWRxPageAncestorsChildren, useSWRxRootPage, mutatePageTree, mutatePageList,
useSWRxRootPage, mutatePageTree, mutatePageList,
} from '~/stores/page-listing';
import { mutateSearching } from '~/stores/search';
import { usePageTreeDescCountMap } from '~/stores/ui';
Expand All @@ -35,66 +34,12 @@ const moduleClass = styles['items-tree'] ?? '';

const logger = loggerFactory('growi:cli:ItemsTree');

/*
* Utility to generate initial node
*/
const generateInitialNodeBeforeResponse = (targetAndAncestors: Partial<IPageHasId>[]): ItemNode => {
const nodes = targetAndAncestors.map((page): ItemNode => {
return new ItemNode(page, []);
});

// update children for each node
const rootNode = nodes.reduce((child, parent) => {
parent.children = [child];
return parent;
});

return rootNode;
};

const generateInitialNodeAfterResponse = (ancestorsChildren: Record<string, Partial<IPageHasId>[]>, rootNode: ItemNode): ItemNode => {
const paths = Object.keys(ancestorsChildren);

let currentNode = rootNode;
paths.every((path) => {
// stop rendering when non-migrated pages found
if (currentNode == null) {
return false;
}

const childPages = ancestorsChildren[path];
currentNode.children = ItemNode.generateNodesFromPages(childPages);
const nextNode = currentNode.children.filter((node) => {
return paths.includes(node.page.path as string);
})[0];
currentNode = nextNode;
return true;
});

return rootNode;
};

// user defined typeguard to assert the arg is not null
type RenderingCondition = {
ancestorsChildrenResult: AncestorsChildrenResult | undefined,
rootPageResult: RootPageResult | undefined,
}
type SecondStageRenderingCondition = {
ancestorsChildrenResult: AncestorsChildrenResult,
rootPageResult: RootPageResult,
}
const isSecondStageRenderingCondition = (condition: RenderingCondition|SecondStageRenderingCondition): condition is SecondStageRenderingCondition => {
return condition.ancestorsChildrenResult != null && condition.rootPageResult != null;
};


type ItemsTreeProps = {
isEnableActions: boolean
isReadOnlyUser: boolean
isWipPageShown?: boolean
targetPath: string
targetPathOrId?: Nullable<string>
targetAndAncestorsData?: TargetAndAncestors
targetPathOrId?: string,
CustomTreeItem: React.FunctionComponent<TreeItemProps>
onClickTreeItem?: (page: IPageForItem) => void;
}
Expand All @@ -104,14 +49,13 @@ type ItemsTreeProps = {
*/
export const ItemsTree = (props: ItemsTreeProps): JSX.Element => {
const {
targetPath, targetPathOrId, targetAndAncestorsData, isEnableActions, isReadOnlyUser, isWipPageShown, CustomTreeItem, onClickTreeItem,
targetPath, targetPathOrId, isEnableActions, isReadOnlyUser, isWipPageShown, CustomTreeItem, onClickTreeItem,
} = props;

const { t } = useTranslation();
const router = useRouter();

const { data: ancestorsChildrenResult, error: error1 } = useSWRxPageAncestorsChildren(targetPath, { suspense: true });
const { data: rootPageResult, error: error2 } = useSWRxRootPage({ suspense: true });
const { data: rootPageResult, error } = useSWRxRootPage({ suspense: true });
const { data: currentPagePath } = useCurrentPagePath();
const { open: openDuplicateModal } = usePageDuplicateModal();
const { open: openDeleteModal } = usePageDeleteModal();
Expand All @@ -122,14 +66,6 @@ export const ItemsTree = (props: ItemsTreeProps): JSX.Element => {
// for mutation
const { trigger: mutateCurrentPage } = useSWRMUTxCurrentPage();


const renderingCondition = useMemo(() => {
return {
ancestorsChildrenResult,
rootPageResult,
};
}, [ancestorsChildrenResult, rootPageResult]);

useEffect(() => {
if (socket == null) {
return;
Expand Down Expand Up @@ -197,34 +133,18 @@ export const ItemsTree = (props: ItemsTreeProps): JSX.Element => {
}, [currentPagePath, mutateCurrentPage, openDeleteModal, router, t]);


if (error1 != null || error2 != null) {
// TODO: improve message
toastError('Error occurred while fetching pages to render PageTree');
if (error != null) {
toastError(t('pagetree.error_retrieving_the_pagetree'));
return <></>;
}

let initialItemNode;
/*
* Render second stage
*/
if (isSecondStageRenderingCondition(renderingCondition)) {
initialItemNode = generateInitialNodeAfterResponse(
renderingCondition.ancestorsChildrenResult.ancestorsChildren,
new ItemNode(renderingCondition.rootPageResult.rootPage),
);
}
/*
* Before swr response comes back
*/
else if (targetAndAncestorsData != null) {
initialItemNode = generateInitialNodeBeforeResponse(targetAndAncestorsData.targetAndAncestors);
}

const initialItemNode = rootPageResult ? new ItemNode(rootPageResult.rootPage) : null;
if (initialItemNode != null) {
return (
<ul className={`${moduleClass} list-group`}>
<CustomTreeItem
key={initialItemNode.page.path}
targetPath={targetPath}
targetPathOrId={targetPathOrId}
itemNode={initialItemNode}
isOpen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
import SimpleBar from 'simplebar-react';

import type { IPageForItem } from '~/interfaces/page';
import { useTargetAndAncestors, useIsGuestUser, useIsReadOnlyUser } from '~/stores-universal/context';
import { useIsGuestUser, useIsReadOnlyUser } from '~/stores-universal/context';
import { usePageSelectModal } from '~/stores/modal';
import { useSWRxCurrentPage } from '~/stores/page';

Expand All @@ -38,7 +38,6 @@ export const PageSelectModal: FC = () => {

const { data: isGuestUser } = useIsGuestUser();
const { data: isReadOnlyUser } = useIsReadOnlyUser();
const { data: targetAndAncestorsData } = useTargetAndAncestors();
const { data: currentPage } = useSWRxCurrentPage();

const pagePathRenameHandler = usePagePathRenameHandler(currentPage);
Expand Down Expand Up @@ -96,7 +95,6 @@ export const PageSelectModal: FC = () => {
isReadOnlyUser={!!isReadOnlyUser}
targetPath={targetPath}
targetPathOrId={targetPathOrId}
targetAndAncestorsData={targetAndAncestorsData}
onClickTreeItem={onClickTreeItem}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import React, {
import { useTranslation } from 'next-i18next';
import { debounce } from 'throttle-debounce';

import { useTargetAndAncestors, useIsGuestUser, useIsReadOnlyUser } from '~/stores-universal/context';
import { useIsGuestUser, useIsReadOnlyUser } from '~/stores-universal/context';
import { useCurrentPagePath, useCurrentPageId } from '~/stores/page';
import {
mutatePageTree, mutateRecentlyUpdated, useSWRxPageAncestorsChildren, useSWRxRootPage, useSWRxV5MigrationStatus,
mutatePageTree, mutateRecentlyUpdated, useSWRxRootPage, useSWRxV5MigrationStatus,
} from '~/stores/page-listing';
import { useSidebarScrollerRef } from '~/stores/ui';
import loggerFactory from '~/utils/logger';
Expand Down Expand Up @@ -99,14 +99,12 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
const { data: isReadOnlyUser } = useIsReadOnlyUser();
const { data: currentPath } = useCurrentPagePath();
const { data: targetId } = useCurrentPageId();
const { data: targetAndAncestorsData } = useTargetAndAncestors();

const { data: migrationStatus } = useSWRxV5MigrationStatus({ suspense: true });

const targetPathOrId = targetId || currentPath;
const path = currentPath || '/';

const { data: ancestorsChildrenResult } = useSWRxPageAncestorsChildren(path, { suspense: true });
const { data: rootPageResult } = useSWRxRootPage({ suspense: true });
const { data: sidebarScrollerRef } = useSidebarScrollerRef();
const [isInitialScrollCompleted, setIsInitialScrollCompleted] = useState(false);
Expand Down Expand Up @@ -144,7 +142,7 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
const scrollOnInitDebounced = useMemo(() => debounce(500, scrollOnInit), [scrollOnInit]);

useEffect(() => {
if (isInitialScrollCompleted || ancestorsChildrenResult == null || rootPageResult == null) {
if (isInitialScrollCompleted || rootPageResult == null) {
return;
}

Expand All @@ -166,7 +164,7 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
return () => {
observer.disconnect();
};
}, [isInitialScrollCompleted, scrollOnInitDebounced, ancestorsChildrenResult, rootPageResult]);
}, [isInitialScrollCompleted, scrollOnInitDebounced, rootPageResult]);
// ******************************* end *******************************


Expand All @@ -189,7 +187,6 @@ export const PageTreeContent = memo(({ isWipPageShown }: PageTreeContentProps) =
isWipPageShown={isWipPageShown}
targetPath={path}
targetPathOrId={targetPathOrId}
targetAndAncestorsData={targetAndAncestorsData}
CustomTreeItem={PageTreeItem}
/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const moduleClass = styles['page-tree-item'] ?? '';

const logger = loggerFactory('growi:cli:Item');

export const PageTreeItem: FC<TreeItemProps> = (props) => {
export const PageTreeItem = (props:TreeItemProps): JSX.Element => {
const router = useRouter();

const getNewPathAfterMoved = (droppedPagePath: string, newParentPagePath: string): string => {
Expand Down Expand Up @@ -186,6 +186,7 @@ export const PageTreeItem: FC<TreeItemProps> = (props) => {
return (
<TreeItemLayout
className={moduleClass}
targetPath={props.targetPath}
targetPathOrId={props.targetPathOrId}
itemLevel={props.itemLevel}
itemNode={props.itemNode}
Expand Down
14 changes: 8 additions & 6 deletions apps/app/src/client/components/TreeItem/TreeItemLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,21 @@ type TreeItemLayoutProps = TreeItemProps & {
indentSize?: number,
}

export const TreeItemLayout: FC<TreeItemLayoutProps> = (props) => {
export const TreeItemLayout = (props: TreeItemLayoutProps): JSX.Element => {
const {
className, itemClassName,
indentSize = 10,
itemLevel: baseItemLevel = 1,
itemNode, targetPathOrId, isOpen: _isOpen = false,
itemNode, targetPath, targetPathOrId, isOpen: _isOpen = false,
onRenamed, onClick, onClickDuplicateMenuItem, onClickDeleteMenuItem, onWheelClick,
isEnableActions, isReadOnlyUser, isWipPageShown = true,
itemRef, itemClass,
showAlternativeContent,
} = props;

const { page, children } = itemNode;
const { page } = itemNode;

const [currentChildren, setCurrentChildren] = useState<ItemNode[]>(children);
const [currentChildren, setCurrentChildren] = useState<ItemNode[]>([]);
const [isOpen, setIsOpen] = useState(_isOpen);

const { data } = useSWRxPageChildren(isOpen ? page._id : null);
Expand Down Expand Up @@ -84,8 +84,9 @@ export const TreeItemLayout: FC<TreeItemLayoutProps> = (props) => {

// didMount
useEffect(() => {
if (hasChildren()) setIsOpen(true);
}, [hasChildren]);
const isPathToTarget = page.path != null && targetPath.startsWith(page.path) && targetPath !== page.path; // Target Page does not need to be opened
if (isPathToTarget) setIsOpen(true);
}, [targetPath, page.path]);

/*
* When swr fetch succeeded
Expand All @@ -108,6 +109,7 @@ export const TreeItemLayout: FC<TreeItemLayoutProps> = (props) => {
isReadOnlyUser,
isOpen: false,
isWipPageShown,
targetPath,
targetPathOrId,
onRenamed,
onClickDuplicateMenuItem,
Expand Down
4 changes: 2 additions & 2 deletions apps/app/src/client/components/TreeItem/interfaces/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { IPageToDeleteWithMeta } from '@growi/core';
import type { Nullable } from 'vitest';

import type { IPageForItem } from '~/interfaces/page';
import type { IPageForPageDuplicateModal } from '~/stores/modal';
Expand All @@ -23,7 +22,8 @@ export type TreeItemToolProps = TreeItemBaseProps & {
};

export type TreeItemProps = TreeItemBaseProps & {
targetPathOrId?: Nullable<string>,
targetPath: string,
targetPathOrId?:string,
isOpen?: boolean,
isWipPageShown?: boolean,
itemClass?: React.FunctionComponent<TreeItemProps>,
Expand Down
7 changes: 0 additions & 7 deletions apps/app/src/interfaces/page-listing-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ export interface ChildrenResult {
children: Partial<IPageForItem>[]
}


export interface TargetAndAncestors {
targetAndAncestors: Partial<IPageForItem>[]
rootPage: Partial<IPageForItem>,
}


export interface V5MigrationStatus {
isV5Compatible : boolean,
migratablePagesCount: number
Expand Down
6 changes: 0 additions & 6 deletions apps/app/src/stores-universal/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import useSWRImmutable from 'swr/immutable';
import type { SupportedActionType } from '~/interfaces/activity';
import type { RendererConfig } from '~/interfaces/services/renderer';

import type { TargetAndAncestors } from '../interfaces/page-listing-results';

import { useContextSWR } from './use-context-swr';

declare global {
Expand Down Expand Up @@ -78,10 +76,6 @@ export const useIsSearchPage = (initialData?: Nullable<boolean>) : SWRResponse<N
return useContextSWR<Nullable<any>, Error>('isSearchPage', initialData);
};

export const useTargetAndAncestors = (initialData?: TargetAndAncestors): SWRResponse<TargetAndAncestors, Error> => {
return useContextSWR<TargetAndAncestors, Error>('targetAndAncestors', initialData);
};

export const useIsAclEnabled = (initialData?: boolean) : SWRResponse<boolean, Error> => {
return useContextSWR<boolean, Error>('isAclEnabled', initialData);
};
Expand Down
Loading
Loading