From b425c9d1a885e63fcad525782f8c3d39e82f9754 Mon Sep 17 00:00:00 2001 From: somebody1234 Date: Wed, 26 Jun 2024 00:13:10 +1000 Subject: [PATCH] Fix opening cloud projects (#10351) - Fix https://github.com/enso-org/cloud-v2/issues/1324 # Important Notes None --- .../AriaComponents/Button/Button.tsx | 33 ++++++++---- .../AriaComponents/Text/useVisualTooltip.tsx | 4 +- .../src/components/dashboard/ProjectIcon.tsx | 54 ++++++++++++------- .../lib/dashboard/src/layouts/Settings.tsx | 4 +- .../lib/dashboard/src/layouts/TabBar.tsx | 23 +++++++- .../src/pages/dashboard/Dashboard.tsx | 37 ++++++------- .../lib/dashboard/src/services/Backend.ts | 8 ++- .../dashboard/src/services/RemoteBackend.ts | 5 +- 8 files changed, 105 insertions(+), 63 deletions(-) diff --git a/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Button/Button.tsx b/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Button/Button.tsx index 521701685c2..28ff330679e 100644 --- a/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Button/Button.tsx +++ b/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Button/Button.tsx @@ -5,7 +5,7 @@ import * as focusHooks from '#/hooks/focusHooks' import * as aria from '#/components/aria' import * as ariaComponents from '#/components/AriaComponents' -import Spinner, * as spinnerModule from '#/components/Spinner' +import StatelessSpinner, * as spinnerModule from '#/components/StatelessSpinner' import SvgMask from '#/components/SvgMask' import * as twv from '#/utilities/tailwindVariants' @@ -62,6 +62,9 @@ export interface BaseButtonProps extends Omit {}, ...ariaProps } = props @@ -326,12 +330,15 @@ export const Button = React.forwardRef(function Button( [{ opacity: 0 }, { opacity: 0, offset: 1 }, { opacity: 1 }], { duration: delay, easing: 'linear', delay: 0, fill: 'forwards' } ) - const contentAnimation = contentRef.current?.animate([{ opacity: 1 }, { opacity: 0 }], { - duration: 0, - easing: 'linear', - delay, - fill: 'forwards', - }) + const contentAnimation = + loaderPosition !== 'full' + ? null + : contentRef.current?.animate([{ opacity: 1 }, { opacity: 0 }], { + duration: 0, + easing: 'linear', + delay, + fill: 'forwards', + }) return () => { loaderAnimation?.cancel() @@ -340,7 +347,7 @@ export const Button = React.forwardRef(function Button( } else { return () => {} } - }, [isLoading]) + }, [isLoading, loaderPosition]) const handlePress = (event: aria.PressEvent): void => { if (!isLoading) { @@ -381,6 +388,12 @@ export const Button = React.forwardRef(function Button( const iconComponent = (() => { if (icon == null) { return null + } else if (isLoading && loaderPosition === 'icon') { + return ( + + + + ) } else if (typeof icon === 'string') { return } else { @@ -425,9 +438,9 @@ export const Button = React.forwardRef(function Button( {childrenFactory()} - {isLoading && ( + {isLoading && loaderPosition === 'full' && ( - + )} diff --git a/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Text/useVisualTooltip.tsx b/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Text/useVisualTooltip.tsx index 8340fea445e..20df4a45dd4 100644 --- a/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Text/useVisualTooltip.tsx +++ b/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Text/useVisualTooltip.tsx @@ -12,8 +12,6 @@ import * as aria from '#/components/aria' import * as ariaComponents from '#/components/AriaComponents' import Portal from '#/components/Portal' -import * as mergeRefs from '#/utilities/mergeRefs' - /** * Props for {@link useVisualTooltip}. */ @@ -123,7 +121,7 @@ export function useVisualTooltip(props: VisualTooltipProps) { const createTooltipElement = () => ( ref?.showPopover())} + ref={popoverRef} {...aria.mergeProps>()( overlayProps, tooltipProps, diff --git a/app/ide-desktop/lib/dashboard/src/components/dashboard/ProjectIcon.tsx b/app/ide-desktop/lib/dashboard/src/components/dashboard/ProjectIcon.tsx index 851bf2c0f4c..6f3c18a6506 100644 --- a/app/ide-desktop/lib/dashboard/src/components/dashboard/ProjectIcon.tsx +++ b/app/ide-desktop/lib/dashboard/src/components/dashboard/ProjectIcon.tsx @@ -113,6 +113,7 @@ export default function ProjectIcon(props: ProjectIconProps) { item.projectState.executeAsync ?? false ) const [shouldSwitchPage, setShouldSwitchPage] = React.useState(false) + const doAbortOpeningRef = React.useRef(() => {}) const doOpenEditorRef = React.useRef(doOpenEditor) doOpenEditorRef.current = doOpenEditor const isCloud = backend.type === backendModule.BackendType.remote @@ -133,14 +134,22 @@ export default function ProjectIcon(props: ProjectIconProps) { mutationKey: ['openEditor', item.id], networkMode: 'always', mutationFn: async () => { - const projectPromise = waitUntilProjectIsReadyMutation.mutateAsync([ - item.id, - item.parentId, - item.title, - ]) - if (shouldOpenWhenReady) { - doOpenEditor() + const abortController = new AbortController() + doAbortOpeningRef.current = () => { + abortController.abort() } + const projectPromise = openProjectMutate([ + item.id, + { executeAsync: false, parentId: item.parentId, cognitoCredentials: session }, + item.title, + ]).then(() => + waitUntilProjectIsReadyMutation.mutateAsync([ + item.id, + item.parentId, + item.title, + abortController.signal, + ]) + ) setProjectStartupInfo({ project: projectPromise, projectAsset: item, @@ -149,6 +158,12 @@ export default function ProjectIcon(props: ProjectIconProps) { accessToken: session?.accessToken ?? null, }) await projectPromise + if (!abortController.signal.aborted) { + setState(backendModule.ProjectState.opened) + if (shouldOpenWhenReady) { + doOpenEditor() + } + } }, }) const openEditorMutate = openEditorMutation.mutate @@ -156,19 +171,21 @@ export default function ProjectIcon(props: ProjectIconProps) { const openProject = React.useCallback( async (shouldRunInBackground: boolean) => { if (state !== backendModule.ProjectState.opened) { - setState(backendModule.ProjectState.openInProgress) try { - await openProjectMutate([ - item.id, - { - executeAsync: shouldRunInBackground, - parentId: item.parentId, - cognitoCredentials: session, - }, - item.title, - ]) if (!shouldRunInBackground) { + setState(backendModule.ProjectState.openInProgress) openEditorMutate() + } else { + setState(backendModule.ProjectState.opened) + await openProjectMutate([ + item.id, + { + executeAsync: shouldRunInBackground, + parentId: item.parentId, + cognitoCredentials: session, + }, + item.title, + ]) } } catch (error) { const project = await getProjectDetailsMutate([item.id, item.parentId, item.title]) @@ -176,7 +193,6 @@ export default function ProjectIcon(props: ProjectIconProps) { // not just the state type. setItem(object.merger({ projectState: project.state })) toastAndLog('openProjectError', error, item.title) - setState(backendModule.ProjectState.closed) } } }, @@ -211,6 +227,7 @@ export default function ProjectIcon(props: ProjectIconProps) { if (!event.runInBackground && !isRunningInBackground) { setShouldOpenWhenReady(false) if (!isOtherUserUsingProject && backendModule.IS_OPENING_OR_OPENED[state]) { + doAbortOpeningRef.current() void closeProject() } } @@ -272,6 +289,7 @@ export default function ProjectIcon(props: ProjectIconProps) { setShouldOpenWhenReady(false) setState(backendModule.ProjectState.closing) await closeProjectMutation.mutateAsync([item.id, item.title]) + setState(backendModule.ProjectState.closed) } switch (state) { diff --git a/app/ide-desktop/lib/dashboard/src/layouts/Settings.tsx b/app/ide-desktop/lib/dashboard/src/layouts/Settings.tsx index 57e9475eabb..ea116a5e90f 100644 --- a/app/ide-desktop/lib/dashboard/src/layouts/Settings.tsx +++ b/app/ide-desktop/lib/dashboard/src/layouts/Settings.tsx @@ -113,9 +113,9 @@ export default function Settings(props: SettingsProps) { /> - + {getText('settingsFor')} - + { readonly isActive: boolean readonly icon: string readonly labelId: text.TextId + /** When the promise is in flight, the tab icon will instead be a loading spinner. */ + readonly loadingPromise?: Promise readonly onPress: () => void readonly onClose?: () => void } /** A tab in a {@link TabBar}. */ export function Tab(props: InternalTabProps) { - const { isActive, icon, labelId, children, onPress, onClose } = props + const { isActive, icon, labelId, loadingPromise, children, onPress, onClose } = props const { updateClipPath } = useTabBarContext() const { getText } = textProvider.useText() + const [isLoading, setIsLoading] = React.useState(loadingPromise != null) + + React.useEffect(() => { + if (loadingPromise) { + setIsLoading(true) + loadingPromise.then( + () => { + setIsLoading(false) + }, + () => { + setIsLoading(false) + } + ) + } else { + setIsLoading(false) + } + }, [loadingPromise]) return (
+ readonly projectStartupInfo: Omit } } @@ -86,15 +85,13 @@ LocalStorage.registerKey('projectStartupInfo', { return null } else if (!('backendType' in value) || !array.includes(BACKEND_TYPES, value.backendType)) { return null - } else if (!('project' in value) || !('projectAsset' in value)) { + } else if (!('projectAsset' in value)) { return null } else { return { // These type assertions are UNSAFE, however correctly type-checking these // would be very complicated. // eslint-disable-next-line no-restricted-syntax - project: value.project as backendModule.Project, - // eslint-disable-next-line no-restricted-syntax projectAsset: value.projectAsset as backendModule.ProjectAsset, backendType: value.backendType, accessToken: value.accessToken ?? null, @@ -143,8 +140,7 @@ export default function Dashboard(props: DashboardProps) { ) const [projectStartupInfo, setProjectStartupInfo] = React.useState(null) - const [openProjectAbortController, setOpenProjectAbortController] = - React.useState(null) + const openProjectAbortControllerRef = React.useRef(null) const [assetListEvents, dispatchAssetListEvent] = eventHooks.useEvent() const [assetEvents, dispatchAssetEvent] = eventHooks.useEvent() @@ -187,7 +183,7 @@ export default function Dashboard(props: DashboardProps) { setPage(TabType.drive) void (async () => { const abortController = new AbortController() - setOpenProjectAbortController(abortController) + openProjectAbortControllerRef.current = abortController try { const oldProject = await remoteBackend.getProjectDetails( savedProjectStartupInfo.projectAsset.id, @@ -199,13 +195,9 @@ export default function Dashboard(props: DashboardProps) { savedProjectStartupInfo.projectAsset.id, savedProjectStartupInfo.projectAsset.parentId, savedProjectStartupInfo.projectAsset.title, - abortController - ) - setProjectStartupInfo( - object.merge(savedProjectStartupInfo, { - project, - }) + abortController.signal ) + setProjectStartupInfo({ ...savedProjectStartupInfo, project }) if (page === TabType.editor) { setPage(page) } @@ -232,9 +224,7 @@ export default function Dashboard(props: DashboardProps) { savedProjectStartupInfo.projectAsset.parentId, savedProjectStartupInfo.projectAsset.title ) - setProjectStartupInfo( - object.merge(savedProjectStartupInfo, { project }) - ) + setProjectStartupInfo({ ...savedProjectStartupInfo, project }) if (page === TabType.editor) { setPage(page) } @@ -249,8 +239,8 @@ export default function Dashboard(props: DashboardProps) { eventHooks.useEventHandler(assetEvents, event => { switch (event.type) { case AssetEventType.openProject: { - openProjectAbortController?.abort() - setOpenProjectAbortController(null) + openProjectAbortControllerRef.current?.abort() + openProjectAbortControllerRef.current = null break } default: { @@ -263,9 +253,10 @@ export default function Dashboard(props: DashboardProps) { React.useEffect(() => { if (initializedRef.current) { if (projectStartupInfo != null) { - void Promise.resolve(projectStartupInfo.project).then(project => { - localStorage.set('projectStartupInfo', { ...projectStartupInfo, project }) - }) + // This is INTENTIONAL - `project` is intentionally omitted from this object. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { project, ...rest } = projectStartupInfo + localStorage.set('projectStartupInfo', rest) } else { localStorage.delete('projectStartupInfo') } @@ -376,6 +367,7 @@ export default function Dashboard(props: DashboardProps) { isActive={page === TabType.editor} icon={WorkspaceIcon} labelId="editorPageName" + loadingPromise={projectStartupInfo.project} onPress={() => { setPage(TabType.editor) }} @@ -384,6 +376,7 @@ export default function Dashboard(props: DashboardProps) { type: AssetEventType.closeProject, id: projectStartupInfo.projectAsset.id, }) + setProjectStartupInfo(null) setPage(TabType.drive) }} > diff --git a/app/ide-desktop/lib/dashboard/src/services/Backend.ts b/app/ide-desktop/lib/dashboard/src/services/Backend.ts index 461251cc1ec..ef31717f5cf 100644 --- a/app/ide-desktop/lib/dashboard/src/services/Backend.ts +++ b/app/ide-desktop/lib/dashboard/src/services/Backend.ts @@ -292,10 +292,8 @@ export interface BackendProject extends Project { } /** Information required to open a project. */ -export interface ProjectStartupInfo< - ProjectType extends Project | Promise = Project | Promise, -> { - readonly project: ProjectType +export interface ProjectStartupInfo { + readonly project: Promise readonly projectAsset: ProjectAsset // This MUST BE optional because it is lost when `JSON.stringify`ing to put in `localStorage`. readonly setProjectAsset?: React.Dispatch> @@ -1428,6 +1426,6 @@ export default abstract class Backend { projectId: ProjectId, directory: DirectoryId | null, title: string, - abortController?: AbortController + abortSignal?: AbortSignal ): Promise } diff --git a/app/ide-desktop/lib/dashboard/src/services/RemoteBackend.ts b/app/ide-desktop/lib/dashboard/src/services/RemoteBackend.ts index bd0592e58aa..5e173d80e86 100644 --- a/app/ide-desktop/lib/dashboard/src/services/RemoteBackend.ts +++ b/app/ide-desktop/lib/dashboard/src/services/RemoteBackend.ts @@ -1067,11 +1067,12 @@ export default class RemoteBackend extends Backend { projectId: backend.ProjectId, directory: backend.DirectoryId | null, title: string, - abortController: AbortController = new AbortController() + abortSignal?: AbortSignal ) { let project = await this.getProjectDetails(projectId, directory, title) while (project.state.type !== backend.ProjectState.opened) { - if (abortController.signal.aborted) { + if (abortSignal?.aborted === true) { + // The operation was cancelled, do not return. // eslint-disable-next-line no-restricted-syntax throw new Error() }