From 5bc873178a5779e41ddb662d40eeb6ee386d5736 Mon Sep 17 00:00:00 2001
From: somebody1234 <ehern.lee@gmail.com>
Date: Mon, 22 Jul 2024 19:40:14 +1000
Subject: [PATCH] Fixes (including downloading Local projects from nested
 directories) (#10585)

- Address part of https://github.com/enso-org/cloud-v2/issues/1350
- Turns out the reason downloads were broken were (I assume) the query string was getting lost - so the Electron server never passed the correct parent directory to the backend.
- Fix "Escape" key using old project ID and navigating to a nonexistent tab
- Fix validation for tab names (previously all strings were passing validation due to an incorrect custom predicate being passed to `zod`)
- Add clip path to entire tab bar so that the bottoms of tabs are cut off on hover if they are next to the currently selected tab.
- Add s-shaped curve to hovered tabs, so that their edges match the edges of the currently selected tab.
- Avoid navigating back to "Data Catalog" page when closing a project tab, when the project tab is not the currently open page.
- Fix size of paywall icons in "Shared With" column (16px to be consistent with all other icons)

# Important Notes
None
---
 .../e2e/actions/EditorPageActions.ts          |  8 ++
 app/dashboard/e2e/copy.spec.ts                |  2 +
 .../dashboard/column/SharedWithColumn.tsx     |  3 +-
 .../columnHeading/SharedWithColumnHeading.tsx |  4 +-
 app/dashboard/src/hooks/projectHooks.ts       | 11 ++-
 app/dashboard/src/layouts/Editor.tsx          |  6 +-
 app/dashboard/src/layouts/TabBar.tsx          | 99 ++++++++++++++-----
 .../src/pages/dashboard/Dashboard.tsx         | 24 ++---
 .../src/providers/ProjectsProvider.tsx        | 19 ++--
 app/ide-desktop/client/src/bin/server.ts      | 12 +--
 10 files changed, 124 insertions(+), 64 deletions(-)

diff --git a/app/dashboard/e2e/actions/EditorPageActions.ts b/app/dashboard/e2e/actions/EditorPageActions.ts
index f87535cca15..6121f2c93ec 100644
--- a/app/dashboard/e2e/actions/EditorPageActions.ts
+++ b/app/dashboard/e2e/actions/EditorPageActions.ts
@@ -12,4 +12,12 @@ export default class EditorPageActions extends PageActions {
   get goToPage(): Omit<goToPageActions.GoToPageActions, 'editor'> {
     return goToPageActions.goToPageActions(this.step.bind(this))
   }
+  /**
+   * Waits for the editor to load.
+   */
+  waitForEditorToLoad(): EditorPageActions {
+    return this.step('wait for the editor to load', async () => {
+      await this.page.waitForSelector('[data-testid=editor]', { state: 'visible' })
+    })
+  }
 }
diff --git a/app/dashboard/e2e/copy.spec.ts b/app/dashboard/e2e/copy.spec.ts
index f14cf38ef32..b1f9839d841 100644
--- a/app/dashboard/e2e/copy.spec.ts
+++ b/app/dashboard/e2e/copy.spec.ts
@@ -159,6 +159,7 @@ test.test('duplicate', ({ page }) =>
     .mockAllAndLogin({ page })
     // Assets: [0: New Project 1]
     .newEmptyProject()
+    .waitForEditorToLoad()
     .goToPage.drive()
     .driveTable.rightClickRow(0)
     .contextMenu.duplicate()
@@ -176,6 +177,7 @@ test.test('duplicate (keyboard)', ({ page }) =>
     .mockAllAndLogin({ page })
     // Assets: [0: New Project 1]
     .newEmptyProject()
+    .waitForEditorToLoad()
     .goToPage.drive()
     .driveTable.clickRow(0)
     .press('Mod+D')
diff --git a/app/dashboard/src/components/dashboard/column/SharedWithColumn.tsx b/app/dashboard/src/components/dashboard/column/SharedWithColumn.tsx
index 7a47534debc..47335ddd3c3 100644
--- a/app/dashboard/src/components/dashboard/column/SharedWithColumn.tsx
+++ b/app/dashboard/src/components/dashboard/column/SharedWithColumn.tsx
@@ -102,7 +102,8 @@ export default function SharedWithColumn(props: SharedWithColumnPropsInternal) {
         <paywall.PaywallDialogButton
           feature="share"
           variant="icon"
-          size="xxsmall"
+          size="medium"
+          tooltipPlacement="left"
           className="opacity-0 group-hover:opacity-100"
           children={false}
         />
diff --git a/app/dashboard/src/components/dashboard/columnHeading/SharedWithColumnHeading.tsx b/app/dashboard/src/components/dashboard/columnHeading/SharedWithColumnHeading.tsx
index d58dbe6b8aa..c232144dd66 100644
--- a/app/dashboard/src/components/dashboard/columnHeading/SharedWithColumnHeading.tsx
+++ b/app/dashboard/src/components/dashboard/columnHeading/SharedWithColumnHeading.tsx
@@ -30,7 +30,7 @@ export default function SharedWithColumnHeading(props: column.AssetColumnHeading
     <div className="flex h-table-row w-full items-center gap-icon-with-text">
       <ariaComponents.Button
         variant="icon"
-        size="xsmall"
+        size="medium"
         icon={PeopleIcon}
         aria-label={getText('sharedWithColumnHide')}
         onPress={() => {
@@ -46,7 +46,7 @@ export default function SharedWithColumnHeading(props: column.AssetColumnHeading
             feature="share"
             variant="icon"
             children={false}
-            size="xsmall"
+            size="medium"
           />
         )}
       </div>
diff --git a/app/dashboard/src/hooks/projectHooks.ts b/app/dashboard/src/hooks/projectHooks.ts
index 2bdeb1afd83..4b04e4c1269 100644
--- a/app/dashboard/src/hooks/projectHooks.ts
+++ b/app/dashboard/src/hooks/projectHooks.ts
@@ -36,8 +36,10 @@ declare module '#/utilities/LocalStorage' {
 
 const PROJECT_SCHEMA = z
   .object({
-    id: z.custom<backendModule.ProjectId>(x => typeof x === 'string'),
-    parentId: z.custom<backendModule.DirectoryId>(x => typeof x === 'string'),
+    id: z.custom<backendModule.ProjectId>(x => typeof x === 'string' && x.startsWith('project-')),
+    parentId: z.custom<backendModule.DirectoryId>(
+      x => typeof x === 'string' && x.startsWith('directory-')
+    ),
     title: z.string(),
     type: z.nativeEnum(backendModule.BackendType),
   })
@@ -329,6 +331,7 @@ export function useCloseProject() {
   const client = reactQuery.useQueryClient()
   const closeProjectMutation = useCloseProjectMutation()
   const removeLaunchedProject = projectsProvider.useRemoveLaunchedProject()
+  const projectsStore = projectsProvider.useProjectsStore()
   const setPage = projectsProvider.useSetPage()
 
   return eventCallbacks.useEventCallback((project: Project) => {
@@ -359,7 +362,9 @@ export function useCloseProject() {
 
     removeLaunchedProject(project.id)
 
-    setPage(projectsProvider.TabType.drive)
+    if (projectsStore.getState().page === project.id) {
+      setPage(projectsProvider.TabType.drive)
+    }
   })
 }
 
diff --git a/app/dashboard/src/layouts/Editor.tsx b/app/dashboard/src/layouts/Editor.tsx
index 156905de8ef..3d61f482c47 100644
--- a/app/dashboard/src/layouts/Editor.tsx
+++ b/app/dashboard/src/layouts/Editor.tsx
@@ -116,7 +116,11 @@ export default function Editor(props: EditorProps) {
   }
 
   return (
-    <div className={twMerge.twJoin('contents', hidden && 'hidden')} data-testvalue={project.id}>
+    <div
+      className={twMerge.twJoin('contents', hidden && 'hidden')}
+      data-testvalue={project.id}
+      data-testid="editor"
+    >
       {(() => {
         if (projectQuery.isError) {
           return (
diff --git a/app/dashboard/src/layouts/TabBar.tsx b/app/dashboard/src/layouts/TabBar.tsx
index 056de24f820..76d3901c8e6 100644
--- a/app/dashboard/src/layouts/TabBar.tsx
+++ b/app/dashboard/src/layouts/TabBar.tsx
@@ -56,7 +56,7 @@ export interface TabBarProps extends Readonly<React.PropsWithChildren> {}
 export default function TabBar(props: TabBarProps) {
   const { children } = props
   const cleanupResizeObserverRef = React.useRef(() => {})
-  const backgroundRef = React.useRef<HTMLDivElement | null>()
+  const backgroundRef = React.useRef<HTMLDivElement | null>(null)
   const selectedTabRef = React.useRef<HTMLElement | null>(null)
   const [resizeObserver] = React.useState(
     () =>
@@ -68,31 +68,51 @@ export default function TabBar(props: TabBarProps) {
   const [updateClipPath] = React.useState(() => {
     return (element: HTMLElement | null) => {
       const backgroundElement = backgroundRef.current
-      if (backgroundElement != null) {
-        if (element == null) {
+      if (backgroundElement) {
+        const rootElement = backgroundElement.parentElement?.parentElement
+        if (!element) {
           backgroundElement.style.clipPath = ''
+          if (rootElement) {
+            rootElement.style.clipPath = ''
+          }
         } else {
           selectedTabRef.current = element
           const bounds = element.getBoundingClientRect()
           const rootBounds = backgroundElement.getBoundingClientRect()
-          const tabLeft = bounds.left - rootBounds.left
-          const tabRight = bounds.right - rootBounds.left
-          const segments = [
+          const tabLeft = bounds.left - rootBounds.left + TAB_RADIUS_PX
+          const tabRight = bounds.right - rootBounds.left - TAB_RADIUS_PX
+          const rightSegments = [
             'M 0 0',
             `L ${rootBounds.width} 0`,
             `L ${rootBounds.width} ${rootBounds.height}`,
             `L ${tabRight + TAB_RADIUS_PX} ${rootBounds.height}`,
             `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${tabRight} ${rootBounds.height - TAB_RADIUS_PX}`,
+          ]
+          const leftSegments = [
+            `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${tabLeft - TAB_RADIUS_PX} ${rootBounds.height}`,
+            `L 0 ${rootBounds.height}`,
+            'Z',
+          ]
+          const segments = [
+            ...rightSegments,
             `L ${tabRight} ${TAB_RADIUS_PX}`,
             `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 0 ${tabRight - TAB_RADIUS_PX} 0`,
             `L ${tabLeft + TAB_RADIUS_PX} 0`,
             `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 0 ${tabLeft} ${TAB_RADIUS_PX}`,
             `L ${tabLeft} ${rootBounds.height - TAB_RADIUS_PX}`,
-            `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${tabLeft - TAB_RADIUS_PX} ${rootBounds.height}`,
-            `L 0 ${rootBounds.height}`,
-            'Z',
+            ...leftSegments,
           ]
           backgroundElement.style.clipPath = `path("${segments.join(' ')}")`
+          const rootSegments = [
+            ...rightSegments,
+            `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${tabRight - TAB_RADIUS_PX} ${rootBounds.height}`,
+            `L ${tabLeft + TAB_RADIUS_PX} ${rootBounds.height}`,
+            `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${tabLeft} ${rootBounds.height - TAB_RADIUS_PX}`,
+            ...leftSegments,
+          ]
+          if (rootElement) {
+            rootElement.style.clipPath = `path("${rootSegments.join(' ')}")`
+          }
         }
       }
     }
@@ -126,14 +146,11 @@ export default function TabBar(props: TabBarProps) {
   }
 
   return (
-    <div className="relative flex grow">
-      <TabBarContext.Provider value={{ setSelectedTab }}>
-        <FocusArea direction="horizontal">
-          {innerProps => (
-            <aria.TabList
-              className="flex h-12 shrink-0 grow cursor-default items-center rounded-full"
-              {...innerProps}
-            >
+    <FocusArea direction="horizontal">
+      {innerProps => (
+        <div className="relative flex grow" {...innerProps}>
+          <TabBarContext.Provider value={{ setSelectedTab }}>
+            <aria.TabList className="flex h-12 shrink-0 grow transition-[clip-path] duration-300">
               <aria.Tab isDisabled>
                 {/* Putting the background in a `Tab` is a hack, but it is required otherwise there
                  * are issues with the ref to the background being detached, resulting in the clip
@@ -143,15 +160,15 @@ export default function TabBar(props: TabBarProps) {
                     backgroundRef.current = element
                     updateResizeObserver(element)
                   }}
-                  className="pointer-events-none absolute inset-0 bg-primary/5"
+                  className="pointer-events-none absolute inset-0 bg-primary/5 transition-[clip-path] duration-300"
                 />
               </aria.Tab>
               {children}
             </aria.TabList>
-          )}
-        </FocusArea>
-      </TabBarContext.Provider>
-    </div>
+          </TabBarContext.Provider>
+        </div>
+      )}
+    </FocusArea>
   )
 }
 
@@ -181,6 +198,33 @@ export function Tab(props: InternalTabProps) {
   const isLoadingRef = React.useRef(true)
   const { getText } = textProvider.useText()
   const actuallyActive = isActive && !isHidden
+  const [resizeObserver] = React.useState(
+    () =>
+      new ResizeObserver(() => {
+        updateClipPath()
+      })
+  )
+
+  const [updateClipPath] = React.useState(() => {
+    return () => {
+      const element = ref.current
+      if (element) {
+        const bounds = element.getBoundingClientRect()
+        const segments = [
+          `M 0 ${bounds.height}`,
+          `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 0 ${TAB_RADIUS_PX} ${bounds.height - TAB_RADIUS_PX}`,
+          `L ${TAB_RADIUS_PX} ${TAB_RADIUS_PX}`,
+          `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${TAB_RADIUS_PX * 2} 0`,
+          `L ${bounds.width - TAB_RADIUS_PX * 2} 0`,
+          `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 1 ${bounds.width - TAB_RADIUS_PX} ${TAB_RADIUS_PX}`,
+          `L ${bounds.width - TAB_RADIUS_PX} ${bounds.height - TAB_RADIUS_PX}`,
+          `A ${TAB_RADIUS_PX} ${TAB_RADIUS_PX} 0 0 0 ${bounds.width} ${bounds.height}`,
+          'Z',
+        ]
+        element.style.clipPath = `path("${segments.join(' ')}")`
+      }
+    }
+  })
 
   React.useLayoutEffect(() => {
     if (actuallyActive && ref.current) {
@@ -209,14 +253,19 @@ export function Tab(props: InternalTabProps) {
       data-testid={props['data-testid']}
       ref={element => {
         ref.current = element
-        if (actuallyActive && element) {
-          setSelectedTab(element)
+        if (element) {
+          if (actuallyActive) {
+            setSelectedTab(element)
+          }
+          resizeObserver.disconnect()
+          resizeObserver.observe(element)
+          updateClipPath()
         }
       }}
       id={id}
       aria-label={getText(labelId)}
       className={tailwindMerge.twMerge(
-        'relative flex h-full items-center gap-3 rounded-t-2xl px-4',
+        'relative -mx-6 flex h-full items-center gap-3 rounded-t-3xl px-10',
         !isActive &&
           'cursor-pointer opacity-50 hover:bg-frame hover:opacity-75 disabled:cursor-not-allowed disabled:opacity-30 [&.disabled]:cursor-not-allowed [&.disabled]:opacity-30',
         isHidden && 'hidden'
diff --git a/app/dashboard/src/pages/dashboard/Dashboard.tsx b/app/dashboard/src/pages/dashboard/Dashboard.tsx
index ec97de1492f..7f98c59bed8 100644
--- a/app/dashboard/src/pages/dashboard/Dashboard.tsx
+++ b/app/dashboard/src/pages/dashboard/Dashboard.tsx
@@ -128,6 +128,7 @@ function DashboardInner(props: DashboardProps) {
     }
   )
 
+  const projectsStore = projectsProvider.useProjectsStore()
   const page = projectsProvider.usePage()
   const launchedProjects = projectsProvider.useLaunchedProjects()
   const selectedProject = launchedProjects.find(p => p.id === page) ?? null
@@ -187,13 +188,12 @@ function DashboardInner(props: DashboardProps) {
         closeModal: () => {
           updateModal(oldModal => {
             if (oldModal == null) {
-              queueMicrotask(() => {
-                setPage(localStorage.get('page') ?? projectsProvider.TabType.drive)
-              })
-              return oldModal
-            } else {
-              return null
+              const currentPage = projectsStore.getState().page
+              if (array.includes(Object.values(projectsProvider.TabType), currentPage)) {
+                setPage(projectsProvider.TabType.drive)
+              }
             }
+            return null
           })
           if (modalRef.current == null) {
             // eslint-disable-next-line no-restricted-syntax
@@ -201,7 +201,7 @@ function DashboardInner(props: DashboardProps) {
           }
         },
       }),
-    [inputBindings, modalRef, localStorage, updateModal, setPage]
+    [inputBindings, modalRef, localStorage, updateModal, setPage, projectsStore]
   )
 
   React.useEffect(() => {
@@ -260,14 +260,14 @@ function DashboardInner(props: DashboardProps) {
   return (
     <Page hideInfoBar hideChat>
       <div
-        className="flex text-xs text-primary"
+        className="flex min-h-full flex-col text-xs text-primary"
         onContextMenu={event => {
           event.preventDefault()
           unsetModal()
         }}
       >
         <aria.Tabs
-          className="relative flex h-screen grow select-none flex-col container-size"
+          className="relative flex min-h-full grow select-none flex-col container-size"
           selectedKey={page}
           onSelectionChange={newPage => {
             const validated = projectsProvider.PAGES_SCHEMA.safeParse(newPage)
@@ -333,7 +333,7 @@ function DashboardInner(props: DashboardProps) {
           <aria.TabPanel
             shouldForceMount
             id={projectsProvider.TabType.drive}
-            className="flex grow [&[data-inert]]:hidden"
+            className="flex min-h-0 grow [&[data-inert]]:hidden"
           >
             <Drive
               assetsManagementApiRef={assetManagementApiRef}
@@ -348,7 +348,7 @@ function DashboardInner(props: DashboardProps) {
               <aria.TabPanel
                 shouldForceMount
                 id={project.id}
-                className="flex grow [&[data-inert]]:hidden"
+                className="flex min-h-0 grow [&[data-inert]]:hidden"
               >
                 <Editor
                   key={project.id}
@@ -380,7 +380,7 @@ function DashboardInner(props: DashboardProps) {
                 />
               </aria.TabPanel>
             ))}
-          <aria.TabPanel id={projectsProvider.TabType.settings} className="flex grow">
+          <aria.TabPanel id={projectsProvider.TabType.settings} className="flex min-h-0 grow">
             <Settings />
           </aria.TabPanel>
         </aria.Tabs>
diff --git a/app/dashboard/src/providers/ProjectsProvider.tsx b/app/dashboard/src/providers/ProjectsProvider.tsx
index c7d86bc02b3..dd8820b936a 100644
--- a/app/dashboard/src/providers/ProjectsProvider.tsx
+++ b/app/dashboard/src/providers/ProjectsProvider.tsx
@@ -12,7 +12,6 @@ import * as searchParamsState from '#/hooks/searchParamsStateHooks'
 import * as localStorageProvider from '#/providers/LocalStorageProvider'
 
 import * as array from '#/utilities/array'
-import LocalStorage from '#/utilities/LocalStorage'
 
 // ===============
 // === TabType ===
@@ -32,15 +31,16 @@ declare module '#/utilities/LocalStorage' {
   /** */
   interface LocalStorageData {
     readonly isAssetPanelVisible: boolean
-    readonly page: z.infer<typeof PAGES_SCHEMA>
   }
 }
 
 export const PAGES_SCHEMA = z
   .nativeEnum(TabType)
-  .or(z.custom<projectHooks.ProjectId>(value => typeof value === 'string'))
-
-LocalStorage.registerKey('page', { schema: PAGES_SCHEMA })
+  .or(
+    z.custom<projectHooks.ProjectId>(
+      value => typeof value === 'string' && value.startsWith('project-')
+    )
+  )
 
 // =====================
 // === ProjectsStore ===
@@ -120,6 +120,7 @@ export default function ProjectsProvider(props: ProjectsProviderProps) {
 function PageSynchronizer() {
   const { localStorage } = localStorageProvider.useLocalStorage()
   const store = useProjectsStore()
+  const providerPage = usePage()
   const providerSetPage = useSetPage()
   const [page, privateSetPage] = searchParamsState.useSearchParamsState(
     'page',
@@ -136,11 +137,9 @@ function PageSynchronizer() {
     providerSetPage(page)
   }, [page, providerSetPage])
 
-  React.useEffect(() =>
-    store.subscribe(state => {
-      privateSetPage(state.page)
-    })
-  )
+  React.useEffect(() => {
+    privateSetPage(providerPage)
+  }, [providerPage, privateSetPage])
 
   React.useEffect(() =>
     store.subscribe(state => {
diff --git a/app/ide-desktop/client/src/bin/server.ts b/app/ide-desktop/client/src/bin/server.ts
index d5f4b967d3d..7d3d36f3253 100644
--- a/app/ide-desktop/client/src/bin/server.ts
+++ b/app/ide-desktop/client/src/bin/server.ts
@@ -200,16 +200,8 @@ export class Server {
             )
             request.pipe(
                 http.request(
-                    // `...actualUrl` does NOT work because `URL` properties are not enumerable.
-                    {
-                        headers: request.headers,
-                        host: actualUrl.host,
-                        hostname: actualUrl.hostname,
-                        method: request.method,
-                        path: actualUrl.pathname,
-                        port: actualUrl.port,
-                        protocol: actualUrl.protocol,
-                    },
+                    actualUrl,
+                    { headers: request.headers, method: request.method },
                     actualResponse => {
                         response.writeHead(
                             // This is SAFE. The documentation says: