From 4472a2a0b049ecd4f45bd146dbf8558183f96191 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Tue, 8 Aug 2023 20:37:53 -0400 Subject: [PATCH] fix: page validation logic (#3626) --- .../src/components/page-detail-editor.tsx | 1 - .../src/components/root-app-sidebar/index.tsx | 2 +- apps/core/src/hooks/use-navigate-helper.ts | 37 ++------- apps/core/src/layouts/workspace-layout.tsx | 9 --- apps/core/src/pages/workspace/detail-page.tsx | 78 +++++++------------ apps/core/src/pages/workspace/index.tsx | 9 ++- .../components/block-suite-editor/index.tsx | 40 +++++----- .../e2e/blocksuite/block-hub.spec.ts | 5 +- 8 files changed, 67 insertions(+), 114 deletions(-) diff --git a/apps/core/src/components/page-detail-editor.tsx b/apps/core/src/components/page-detail-editor.tsx index 62dcf0f6db..896856528f 100644 --- a/apps/core/src/components/page-detail-editor.tsx +++ b/apps/core/src/components/page-detail-editor.tsx @@ -76,7 +76,6 @@ const EditorWrapper = memo(function EditorWrapper({ '--affine-font-family': value, } as CSSProperties } - key={`${workspace.id}-${pageId}`} mode={isPublic ? 'page' : currentMode} page={page} onInit={useCallback( diff --git a/apps/core/src/components/root-app-sidebar/index.tsx b/apps/core/src/components/root-app-sidebar/index.tsx index a6167dc539..1e202203f2 100644 --- a/apps/core/src/components/root-app-sidebar/index.tsx +++ b/apps/core/src/components/root-app-sidebar/index.tsx @@ -115,7 +115,7 @@ export const RootAppSidebar = ({ const [sidebarOpen, setSidebarOpen] = useAtom(appSidebarOpenAtom); useEffect(() => { - if (isDesktop && typeof sidebarOpen === 'boolean') { + if (isDesktop) { window.apis?.ui.handleSidebarVisibilityChange(sidebarOpen).catch(err => { console.error(err); }); diff --git a/apps/core/src/hooks/use-navigate-helper.ts b/apps/core/src/hooks/use-navigate-helper.ts index 9f74607452..1a85b0bbb6 100644 --- a/apps/core/src/hooks/use-navigate-helper.ts +++ b/apps/core/src/hooks/use-navigate-helper.ts @@ -1,9 +1,4 @@ import type { WorkspaceSubPath } from '@affine/env/workspace'; -import { - currentPageIdAtom, - currentWorkspaceIdAtom, -} from '@toeverything/infra/atom'; -import { useSetAtom } from 'jotai'; import { useCallback } from 'react'; // eslint-disable-next-line @typescript-eslint/no-restricted-imports import { useLocation, useNavigate } from 'react-router-dom'; @@ -16,8 +11,6 @@ export enum RouteLogic { export function useNavigateHelper() { const location = useLocation(); const navigate = useNavigate(); - const setWorkspaceId = useSetAtom(currentWorkspaceIdAtom); - const setCurrentPageId = useSetAtom(currentPageIdAtom); const jumpToPage = useCallback( ( @@ -25,13 +18,11 @@ export function useNavigateHelper() { pageId: string, logic: RouteLogic = RouteLogic.PUSH ) => { - setWorkspaceId(workspaceId); - setCurrentPageId(pageId); return navigate(`/workspace/${workspaceId}/${pageId}`, { replace: logic === RouteLogic.REPLACE, }); }, - [navigate, setCurrentPageId, setWorkspaceId] + [navigate] ); const jumpToPublicWorkspacePage = useCallback( ( @@ -39,13 +30,11 @@ export function useNavigateHelper() { pageId: string, logic: RouteLogic = RouteLogic.PUSH ) => { - setWorkspaceId(workspaceId); - setCurrentPageId(pageId); return navigate(`/public-workspace/${workspaceId}/${pageId}`, { replace: logic === RouteLogic.REPLACE, }); }, - [navigate, setCurrentPageId, setWorkspaceId] + [navigate] ); const jumpToSubPath = useCallback( ( @@ -53,18 +42,14 @@ export function useNavigateHelper() { subPath: WorkspaceSubPath, logic: RouteLogic = RouteLogic.PUSH ) => { - setWorkspaceId(workspaceId); - setCurrentPageId(null); return navigate(`/workspace/${workspaceId}/${subPath}`, { replace: logic === RouteLogic.REPLACE, }); }, - [navigate, setCurrentPageId, setWorkspaceId] + [navigate] ); const openPage = useCallback( (workspaceId: string, pageId: string) => { - setWorkspaceId(workspaceId); - setCurrentPageId(pageId); const isPublicWorkspace = location.pathname.indexOf('/public-workspace') === 0; if (isPublicWorkspace) { @@ -73,35 +58,25 @@ export function useNavigateHelper() { return jumpToPage(workspaceId, pageId); } }, - [ - jumpToPage, - jumpToPublicWorkspacePage, - location.pathname, - setCurrentPageId, - setWorkspaceId, - ] + [jumpToPage, jumpToPublicWorkspacePage, location.pathname] ); const jumpToIndex = useCallback( (logic: RouteLogic = RouteLogic.PUSH) => { - setWorkspaceId(null); - setCurrentPageId(null); return navigate('/', { replace: logic === RouteLogic.REPLACE, }); }, - [navigate, setCurrentPageId, setWorkspaceId] + [navigate] ); const jumpTo404 = useCallback( (logic: RouteLogic = RouteLogic.PUSH) => { - setWorkspaceId(null); - setCurrentPageId(null); return navigate('/404', { replace: logic === RouteLogic.REPLACE, }); }, - [navigate, setCurrentPageId, setWorkspaceId] + [navigate] ); return { diff --git a/apps/core/src/layouts/workspace-layout.tsx b/apps/core/src/layouts/workspace-layout.tsx index dbffa03e4e..26c9e20ee4 100644 --- a/apps/core/src/layouts/workspace-layout.tsx +++ b/apps/core/src/layouts/workspace-layout.tsx @@ -89,15 +89,6 @@ export const QuickSearch = () => { ); }; -declare global { - // eslint-disable-next-line no-var - var HALTING_PROBLEM_TIMEOUT: number; -} - -if (globalThis.HALTING_PROBLEM_TIMEOUT === undefined) { - globalThis.HALTING_PROBLEM_TIMEOUT = 1000; -} - const showList: IslandItemNames[] = environment.isDesktop ? ['whatNew', 'contact', 'guide'] : ['whatNew', 'contact']; diff --git a/apps/core/src/pages/workspace/detail-page.tsx b/apps/core/src/pages/workspace/detail-page.tsx index f8af18d15c..c1c7fbf229 100644 --- a/apps/core/src/pages/workspace/detail-page.tsx +++ b/apps/core/src/pages/workspace/detail-page.tsx @@ -7,12 +7,16 @@ import { WorkspaceSubPath } from '@affine/env/workspace'; import type { EditorContainer } from '@blocksuite/editor'; import { assertExists } from '@blocksuite/global/utils'; import type { Page } from '@blocksuite/store'; -import { currentPageIdAtom, rootStore } from '@toeverything/infra/atom'; +import { + currentPageIdAtom, + currentWorkspaceAtom, + currentWorkspaceIdAtom, + rootStore, +} from '@toeverything/infra/atom'; import { useAtomValue } from 'jotai'; -import { useAtom } from 'jotai/react'; -import { type ReactElement, useCallback, useEffect } from 'react'; +import { type ReactElement, useCallback } from 'react'; import type { LoaderFunction } from 'react-router-dom'; -import { useLocation, useParams } from 'react-router-dom'; +import { redirect } from 'react-router-dom'; import { getUIAdapter } from '../../adapters/workspace'; import { useCurrentWorkspace } from '../../hooks/current/use-current-workspace'; @@ -32,7 +36,7 @@ const DetailPageImpl = (): ReactElement => { return openPage(blockSuiteWorkspace.id, pageId); }); const disposeTagClick = editor.slots.tagClicked.on(async ({ tagId }) => { - await jumpToSubPath(currentWorkspace.id, WorkspaceSubPath.ALL); + jumpToSubPath(currentWorkspace.id, WorkspaceSubPath.ALL); collectionManager.backToAll(); collectionManager.setTemporaryFilter([createTagFilter(tagId)]); }); @@ -69,61 +73,39 @@ const DetailPageImpl = (): ReactElement => { }; export const DetailPage = (): ReactElement => { - const { workspaceId, pageId } = useParams(); - const location = useLocation(); - const { jumpTo404 } = useNavigateHelper(); const [currentWorkspace] = useCurrentWorkspace(); - const [currentPageId, setCurrentPageId] = useAtom(currentPageIdAtom); + const currentPageId = useAtomValue(currentPageIdAtom); const page = currentPageId ? currentWorkspace.blockSuiteWorkspace.getPage(currentPageId) : null; - //#region check if page is valid - useEffect(() => { - // if the workspace changed, ignore the page check - if (currentWorkspace.id !== workspaceId) { - return; - } - if (typeof pageId === 'string' && currentPageId) { - if (currentPageId !== pageId) { - setCurrentPageId(pageId); - } else { - const page = - currentWorkspace.blockSuiteWorkspace.getPage(currentPageId); - if (!page) { - jumpTo404(); - } else { - // fixme: cleanup jumpOnce in the right time - if (page.meta.jumpOnce) { - currentWorkspace.blockSuiteWorkspace.setPageMeta(currentPageId, { - jumpOnce: false, - }); - } - } - } - } - }, [ - currentPageId, - currentWorkspace.blockSuiteWorkspace, - currentWorkspace.id, - jumpTo404, - location.pathname, - pageId, - setCurrentPageId, - workspaceId, - ]); - //#endregion - if (!currentPageId || !page) { return ; } return ; }; -export const loader: LoaderFunction = args => { +export const loader: LoaderFunction = async args => { + if (args.params.workspaceId) { + localStorage.setItem('last_workspace_id', args.params.workspaceId); + rootStore.set(currentWorkspaceIdAtom, args.params.workspaceId); + } if (args.params.pageId) { - localStorage.setItem('last_page_id', args.params.pageId); - rootStore.set(currentPageIdAtom, args.params.pageId); + const pageId = args.params.pageId; + localStorage.setItem('last_page_id', pageId); + const currentWorkspace = await rootStore.get(currentWorkspaceAtom); + const page = currentWorkspace.getPage(pageId); + if (!page) { + return redirect('/404'); + } + if (page.meta.jumpOnce) { + currentWorkspace.setPageMeta(page.id, { + jumpOnce: false, + }); + } + rootStore.set(currentPageIdAtom, pageId); + } else { + return redirect('/404'); } return null; }; diff --git a/apps/core/src/pages/workspace/index.tsx b/apps/core/src/pages/workspace/index.tsx index d873286057..6c605ebec4 100644 --- a/apps/core/src/pages/workspace/index.tsx +++ b/apps/core/src/pages/workspace/index.tsx @@ -1,5 +1,9 @@ import { rootWorkspacesMetadataAtom } from '@affine/workspace/atom'; -import { currentWorkspaceIdAtom, rootStore } from '@toeverything/infra/atom'; +import { + currentPageIdAtom, + currentWorkspaceIdAtom, + rootStore, +} from '@toeverything/infra/atom'; import type { ReactElement } from 'react'; import { type LoaderFunction, Outlet, redirect } from 'react-router-dom'; @@ -14,6 +18,9 @@ export const loader: LoaderFunction = async args => { localStorage.setItem('last_workspace_id', args.params.workspaceId); rootStore.set(currentWorkspaceIdAtom, args.params.workspaceId); } + if (!args.params.pageId) { + rootStore.set(currentPageIdAtom, null); + } return null; }; diff --git a/packages/component/src/components/block-suite-editor/index.tsx b/packages/component/src/components/block-suite-editor/index.tsx index c097378661..b30be9edfa 100644 --- a/packages/component/src/components/block-suite-editor/index.tsx +++ b/packages/component/src/components/block-suite-editor/index.tsx @@ -95,32 +95,34 @@ const BlockSuiteEditorImpl = (props: EditorProps): ReactElement => { if (!container) { return; } - if (page.awarenessStore.getFlag('enable_block_hub')) { - editor - .createBlockHub() - .then(blockHub => { - if (blockHubRef.current) { - blockHubRef.current.remove(); - } - blockHubRef.current = blockHub; - if (setBlockHub) { - setBlockHub(blockHub); - } - }) - .catch(err => { - console.error(err); - }); - } - container.appendChild(editor); + return () => { + container.removeChild(editor); + }; + }, [editor]); + + useEffect(() => { + editor + .createBlockHub() + .then(blockHub => { + if (blockHubRef.current) { + blockHubRef.current.remove(); + } + blockHubRef.current = blockHub; + if (setBlockHub) { + setBlockHub(blockHub); + } + }) + .catch(err => { + console.error(err); + }); return () => { if (setBlockHub) { setBlockHub(null); } blockHubRef.current?.remove(); - container.removeChild(editor); }; - }, [editor, setBlockHub, page]); + }, [editor, page.awarenessStore, setBlockHub]); // issue: https://github.com/toeverything/AFFiNE/issues/2004 const className = `editor-wrapper ${editor.mode}-mode ${ diff --git a/tests/affine-local/e2e/blocksuite/block-hub.spec.ts b/tests/affine-local/e2e/blocksuite/block-hub.spec.ts index 69750ff280..4109a90955 100644 --- a/tests/affine-local/e2e/blocksuite/block-hub.spec.ts +++ b/tests/affine-local/e2e/blocksuite/block-hub.spec.ts @@ -1,13 +1,10 @@ import { test } from '@affine-test/kit/playwright'; import { checkBlockHub } from '@affine-test/kit/utils/editor'; import { openHomePage } from '@affine-test/kit/utils/load-page'; -import { newPage, waitEditorLoad } from '@affine-test/kit/utils/page-logic'; +import { waitEditorLoad } from '@affine-test/kit/utils/page-logic'; test('block-hub should work', async ({ page }) => { await openHomePage(page); await waitEditorLoad(page); await checkBlockHub(page); - await newPage(page); - await page.waitForTimeout(500); - await checkBlockHub(page); });