Bug fix: make "phantom side panes" get properly cleared (#3878)

* Write a failing test

* Make `openPanes` get cleared of any hidden panes

* Grrr fmt

e2e/playwright/testing-settings.spec.ts

@@ -9,7 +9,11 @@ import {
   executorInputPath,
 } from './test-utils'
 import { SaveSettingsPayload, SettingsLevel } from 'lib/settings/settingsTypes'
-import { TEST_SETTINGS_KEY, TEST_SETTINGS_CORRUPTED } from './storageStates'
+import {
+  TEST_SETTINGS_KEY,
+  TEST_SETTINGS_CORRUPTED,
+  TEST_SETTINGS,
+} from './storageStates'
 import * as TOML from '@iarna/toml'
 
 test.beforeEach(async ({ context, page }) => {
@@ -637,4 +641,82 @@ const extrude001 = extrude(5, sketch001)
         .toBeLessThan(15)
     })
   })
+
+  test(`Turning off "Show debug panel" with debug panel open leaves no phantom panel`, async ({
+    page,
+  }) => {
+    const u = await getUtils(page)
+
+    // Override beforeEach test setup
+    // with debug panel open
+    // but "show debug panel" set to false
+    await page.addInitScript(
+      async ({ settingsKey, settings }) => {
+        localStorage.setItem(settingsKey, settings)
+        localStorage.setItem(
+          'persistModelingContext',
+          '{"openPanes":["debug"]}'
+        )
+      },
+      {
+        settingsKey: TEST_SETTINGS_KEY,
+        settings: TOML.stringify({
+          settings: {
+            ...TEST_SETTINGS,
+            modeling: { ...TEST_SETTINGS.modeling, showDebugPanel: false },
+          },
+        }),
+      }
+    )
+    await page.setViewportSize({ width: 1200, height: 500 })
+
+    // Constants and locators
+    const resizeHandle = page.locator('.sidebar-resize-handles > div.block')
+    const debugPaneButton = page.getByTestId('debug-pane-button')
+    const commandsButton = page.getByRole('button', { name: 'Commands' })
+    const debugPaneOption = page.getByRole('option', {
+      name: 'Settings · modeling · show debug panel',
+    })
+
+    async function setShowDebugPanelTo(value: 'On' | 'Off') {
+      await commandsButton.click()
+      await debugPaneOption.click()
+      await page.getByRole('option', { name: value }).click()
+      await expect(
+        page.getByText(
+          `Set show debug panel to "${value === 'On'}" for this project`
+        )
+      ).toBeVisible()
+    }
+
+    await test.step(`Initial load with corrupted settings`, async () => {
+      await u.waitForAuthSkipAppStart()
+      // Check that the debug panel is not visible
+      await expect(debugPaneButton).not.toBeVisible()
+      // Check the pane resize handle wrapper is not visible
+      await expect(resizeHandle).not.toBeVisible()
+    })
+
+    await test.step(`Open code pane to verify we see the resize handles`, async () => {
+      await u.openKclCodePanel()
+      await expect(resizeHandle).toBeVisible()
+      await u.closeKclCodePanel()
+    })
+
+    await test.step(`Turn on debug panel, open it`, async () => {
+      await setShowDebugPanelTo('On')
+      await expect(debugPaneButton).toBeVisible()
+      // We want the logic to clear the phantom panel, so we shouldn't see
+      // the real panel (and therefore the resize handle) yet
+      await expect(resizeHandle).not.toBeVisible()
+      await u.openDebugPanel()
+      await expect(resizeHandle).toBeVisible()
+    })
+
+    await test.step(`Turn off debug panel setting with it open`, async () => {
+      await setShowDebugPanelTo('Off')
+      await expect(debugPaneButton).not.toBeVisible()
+      await expect(resizeHandle).not.toBeVisible()
+    })
+  })
 })

src/components/ModelingSidebar/ModelingPanes/index.ts

@@ -15,6 +15,8 @@ import { DebugPane } from './DebugPane'
 import { FileTreeInner, FileTreeMenu } from 'components/FileTree'
 import { useKclContext } from 'lang/KclProvider'
 import { editorManager } from 'lib/singletons'
+import { ContextFrom } from 'xstate'
+import { settingsMachine } from 'machines/settingsMachine'
 
 export type SidebarType =
   | 'code'
@@ -36,6 +38,8 @@ export interface BadgeInfo {
  */
 interface PaneCallbackProps {
   kclContext: ReturnType<typeof useKclContext>
+  settings: ContextFrom<typeof settingsMachine>
+  platform: 'web' | 'desktop'
 }
 
 export type SidebarPane = {
@@ -45,10 +49,21 @@ export type SidebarPane = {
   keybinding: string
   Content: ReactNode | React.FC
   Menu?: ReactNode | React.FC
-  hideOnPlatform?: 'desktop' | 'web'
+  hide?: boolean | ((props: PaneCallbackProps) => boolean)
   showBadge?: BadgeInfo
 }
 
+export type SidebarAction = {
+  id: string
+  title: string
+  icon: CustomIconName
+  iconClassName?: string // Just until we get rid of FontAwesome icons
+  keybinding: string
+  action: () => void
+  hide?: boolean | ((props: PaneCallbackProps) => boolean)
+  disable?: () => string | undefined
+}
+
 export const sidebarPanes: SidebarPane[] = [
   {
     id: 'code',
@@ -74,7 +89,7 @@ export const sidebarPanes: SidebarPane[] = [
     Content: FileTreeInner,
     keybinding: 'Shift + F',
     Menu: FileTreeMenu,
-    hideOnPlatform: 'web',
+    hide: ({ platform }) => platform === 'web',
   },
   {
     id: 'variables',
@@ -97,5 +112,6 @@ export const sidebarPanes: SidebarPane[] = [
     icon: faBugSlash,
     Content: DebugPane,
     keybinding: 'Shift + D',
+    hide: ({ settings }) => !settings.modeling.showDebugPanel.current,
   },
 ]

src/components/ModelingSidebar/ModelingSidebar.tsx

@@ -1,8 +1,8 @@
 import { useSettingsAuthContext } from 'hooks/useSettingsAuthContext'
 import { Resizable } from 're-resizable'
-import { MouseEventHandler, useCallback, useMemo } from 'react'
+import { MouseEventHandler, useCallback, useEffect, useMemo } from 'react'
 import { useHotkeys } from 'react-hotkeys-hook'
-import { SidebarType, sidebarPanes } from './ModelingPanes'
+import { SidebarAction, SidebarType, sidebarPanes } from './ModelingPanes'
 import Tooltip from 'components/Tooltip'
 import { ActionIcon } from 'components/ActionIcon'
 import styles from './ModelingSidebar.module.css'
@@ -24,6 +24,10 @@ interface BadgeInfoComputed {
   onClick?: MouseEventHandler<any>
 }
 
+function getPlatformString(): 'web' | 'desktop' {
+  return isDesktop() ? 'desktop' : 'web'
+}
+
 export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
   const { commandBarSend } = useCommandsContext()
   const kclContext = useKclContext()
@@ -37,6 +41,15 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
       : 'pointer-events-auto '
   const showDebugPanel = settings.context.modeling.showDebugPanel
 
+  const paneCallbackProps = useMemo(
+    () => ({
+      kclContext,
+      settings: settings.context,
+      platform: getPlatformString(),
+    }),
+    [kclContext.errors, settings.context]
+  )
+
   const sidebarActions: SidebarAction[] = [
     {
       id: 'export',
@@ -71,11 +84,8 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
   ]
   const filteredActions: SidebarAction[] = sidebarActions.filter(
     (action) =>
-      (!action.hide || (action.hide instanceof Function && !action.hide())) &&
+      !action.hide ||
-      (!action.hideOnPlatform ||
+      (action.hide instanceof Function && !action.hide(paneCallbackProps))
-        (isDesktop()
-          ? action.hideOnPlatform === 'web'
-          : action.hideOnPlatform === 'desktop'))
   )
 
   //   // Filter out the debug panel if it's not supposed to be shown
@@ -87,25 +97,47 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
         : sidebarPanes.filter((pane) => pane.id !== 'debug')
       ).filter(
         (pane) =>
-          !pane.hideOnPlatform ||
+          !pane.hide ||
-          (isDesktop()
+          (pane.hide instanceof Function && !pane.hide(paneCallbackProps))
-            ? pane.hideOnPlatform === 'web'
-            : pane.hideOnPlatform === 'desktop')
       ),
-    [sidebarPanes, showDebugPanel.current]
+    [sidebarPanes, paneCallbackProps]
   )
 
   const paneBadgeMap: Record<SidebarType, BadgeInfoComputed> = useMemo(() => {
     return filteredPanes.reduce((acc, pane) => {
       if (pane.showBadge) {
         acc[pane.id] = {
-          value: pane.showBadge.value({ kclContext }),
+          value: pane.showBadge.value(paneCallbackProps),
           onClick: pane.showBadge.onClick,
         }
       }
       return acc
     }, {} as Record<SidebarType, BadgeInfoComputed>)
-  }, [kclContext.errors])
+  }, [paneCallbackProps])
+
+  // Clear any hidden panes from the `openPanes` array
+  useEffect(() => {
+    const panesToReset: SidebarType[] = []
+    sidebarPanes.forEach((pane) => {
+      if (
+        pane.hide === true ||
+        (pane.hide instanceof Function && pane.hide(paneCallbackProps))
+      ) {
+        panesToReset.push(pane.id)
+      }
+    })
+
+    if (panesToReset.length > 0) {
+      send({
+        type: 'Set context',
+        data: {
+          openPanes: context.store?.openPanes.filter(
+            (pane) => !panesToReset.includes(pane)
+          ),
+        },
+      })
+    }
+  }, [settings.context])
 
   const togglePane = useCallback(
     (newPane: SidebarType) => {
@@ -130,6 +162,7 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
       }}
       minWidth={200}
       maxWidth={800}
+      handleWrapperClass="sidebar-resize-handles"
       handleClasses={{
         right:
           (context.store?.openPanes.length === 0 ? 'hidden ' : 'block ') +
@@ -324,15 +357,3 @@ function ModelingPaneButton({
     </div>
   )
 }
-
-export type SidebarAction = {
-  id: string
-  title: string
-  icon: CustomIconName
-  iconClassName?: string // Just until we get rid of FontAwesome icons
-  keybinding: string
-  action: () => void
-  hideOnPlatform?: 'desktop' | 'web'
-  hide?: boolean | (() => boolean)
-  disable?: () => string | undefined
-}