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
2024-12-22 15:21:43 +03:00 · 2024-07-22 19:40:14 +10:00 · 2024-07-22 19:40:14 +10:00 · 5bc873178a
commit 5bc873178a
parent 5066b5764e
10 changed files with 124 additions and 64 deletions
--- 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' })
+    })
+  }
 }
--- 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')
--- 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}
        />
--- 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>
--- 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)

+    if (projectsStore.getState().page === project.id) {
      setPage(projectsProvider.TabType.drive)
+    }
  })
 }

--- 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 (
--- 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}
-            >
+        <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>
+      )}
+    </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) {
+        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'
--- 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>
--- 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 => {
--- 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: