Replace global functions with Redux state for mobile tab switching

Co-authored-by: thomasnordquist <7721625+thomasnordquist@users.noreply.github.com>

app/src/actions/Global.ts

@@ -53,3 +53,8 @@ export const removeConfirmationRequest = (confirmationRequest: ConfirmationReque
     })
   })
 }
+
+export const setMobileTab = (tabIndex: number) => ({
+  mobileTab: tabIndex,
+  type: ActionTypes.setMobileTab,
+})

app/src/actions/Tree.ts

@@ -12,6 +12,8 @@ export { clearTopic } from './clearTopic'
 
 export { moveSelectionUpOrDownwards, moveInward, moveOutward } from './visibleTreeTraversal'
 
+export { setMobileTab } from './Global'
+
 export const selectTopic =
   (topic: q.TreeNode<TopicViewModel>) => (dispatch: Dispatch<any>, getState: () => AppState) => {
     debouncedSelectTopic(topic, dispatch, getState)

app/src/components/Layout/ContentView.tsx

@@ -10,6 +10,8 @@ import { Sidebar } from '../Sidebar'
 import { useResizeDetector } from 'react-resize-detector'
 import MobileTabs from './MobileTabs'
 import PublishTab from '../Sidebar/PublishTab'
+import { setMobileTab } from '../../actions/Global'
+import { Dispatch } from 'redux'
 
 // Type cast to any to work around React 18 compatibility issues with react-split-pane 0.1.x
 const ReactSplitPane = ReactSplitPaneImport as any
@@ -19,13 +21,14 @@ interface Props {
   paneDefaults: any
   connectionId?: string
   chartPanelItems: List<ChartParameters>
+  mobileTab: number
+  dispatch: Dispatch<any>
 }
 
 function ContentView(props: Props) {
   // Use different defaults for mobile viewports (<=768px width)
   // Use state for mobile detection that updates on resize
   const [isMobile, setIsMobile] = React.useState(() => typeof window !== 'undefined' && window.innerWidth <= 768)
-  const [mobileTab, setMobileTab] = React.useState(0) // 0 = topics, 1 = details, 2 = publish, 3 = charts
   const [height, setHeight] = React.useState<string | number>('100%')
   const [sidebarWidth, setSidebarWidth] = React.useState<string | number>(isMobile ? '100%' : '40%')
   const [detectedHeight, setDetectedHeight] = React.useState(0)
@@ -89,23 +92,6 @@ function ContentView(props: Props) {
 
   // Mobile view with tab switcher
   if (isMobile) {
-    // Expose tab switching functions for other components to call
-    React.useEffect(() => {
-      if (typeof window !== 'undefined') {
-        (window as any).switchToDetailsTab = () => setMobileTab(1)
-        (window as any).switchToTopicsTab = () => setMobileTab(0)
-        ;(window as any).switchToPublishTab = () => setMobileTab(2)
-        ;(window as any).switchToChartsTab = () => setMobileTab(3)
-      }
-      return () => {
-        if (typeof window !== 'undefined') {
-          delete (window as any).switchToDetailsTab
-          delete (window as any).switchToTopicsTab
-          delete (window as any).switchToPublishTab
-          delete (window as any).switchToChartsTab
-        }
-      }
-    }, [])
 
     const mobileContainerStyle: React.CSSProperties = {
       display: 'flex',
@@ -148,28 +134,28 @@ function ContentView(props: Props) {
 
     return (
       <div style={mobileContainerStyle}>
-        <MobileTabs value={mobileTab} onChange={setMobileTab} />
+        <MobileTabs value={props.mobileTab} onChange={(tab) => props.dispatch(setMobileTab(tab))} />
         <div style={tabContentStyle}>
           {/* Topics tab */}
-          {mobileTab === 0 && (
+          {props.mobileTab === 0 && (
             <div style={treeContainerStyle}>
               <Tree />
             </div>
           )}
           {/* Details tab */}
-          {mobileTab === 1 && (
+          {props.mobileTab === 1 && (
             <div style={sidebarContainerStyle}>
               <Sidebar connectionId={props.connectionId} />
             </div>
           )}
           {/* Publish tab */}
-          {mobileTab === 2 && (
+          {props.mobileTab === 2 && (
             <div style={sidebarContainerStyle}>
               <PublishTab connectionId={props.connectionId} />
             </div>
           )}
           {/* Charts tab */}
-          {mobileTab === 3 && (
+          {props.mobileTab === 3 && (
             <div style={sidebarContainerStyle}>
               <ChartPanel />
             </div>
@@ -240,6 +226,7 @@ function ContentView(props: Props) {
 const mapStateToProps = (state: AppState) => {
   return {
     chartPanelItems: state.charts.get('charts'),
+    mobileTab: state.globalState.get('mobileTab'),
   }
 }
 

app/src/components/Layout/SearchBar.tsx

@@ -5,7 +5,7 @@ import { AppState } from '../../reducers'
 import { bindActionCreators } from 'redux'
 import { connect } from 'react-redux'
 import { InputBase } from '@mui/material'
-import { settingsActions } from '../../actions'
+import { settingsActions, globalActions } from '../../actions'
 import { alpha as fade, Theme } from '@mui/material/styles'
 import { withStyles } from '@mui/styles'
 import { useGlobalKeyEventHandler } from '../../effects/useGlobalKeyEventHandler'
@@ -17,6 +17,7 @@ function SearchBar(props: {
   hasConnection: boolean
   actions: {
     settings: typeof settingsActions
+    global: typeof globalActions
   }
 }) {
   const { actions, classes, hasConnection, topicFilter } = props
@@ -27,11 +28,9 @@ function SearchBar(props: {
     setHasFocus(true)
     // On mobile, switch to Topics tab when search is focused
     if (typeof window !== 'undefined' && window.innerWidth <= 768) {
-      if ((window as any).switchToTopicsTab) {
+      actions.global.setMobileTab(0)
-        (window as any).switchToTopicsTab()
     }
-    }
+  }, [actions])
-  }, [])
   const onBlur = useCallback(() => setHasFocus(false), [])
 
   const clearFilter = useCallback(() => {
@@ -101,6 +100,7 @@ const mapDispatchToProps = (dispatch: any) => {
   return {
     actions: {
       settings: bindActionCreators(settingsActions, dispatch),
+      global: bindActionCreators(globalActions, dispatch),
     },
   }
 }

app/src/components/Tree/TreeNode/index.tsx

@@ -69,16 +69,14 @@ function TreeNodeComponent(props: Props) {
         // Expanding is handled by the separate expand button click
         didSelectTopic()
         // Switch to details tab on mobile after selecting a topic
-        if (typeof window !== 'undefined' && (window as any).switchToDetailsTab) {
+        actions.setMobileTab(1)
-          (window as any).switchToDetailsTab()
-        }
       } else {
         // Desktop: Original behavior - select AND toggle (click anywhere works)
         didSelectTopic()
         setCollapsedOverride(!isCollapsed)
       }
     },
-    [isCollapsed, didSelectTopic]
+    [isCollapsed, didSelectTopic, actions]
   )
 
   const toggleCollapsed = useCallback(

app/src/reducers/Global.ts

@@ -12,6 +12,7 @@ export enum ActionTypes {
   requestConfirmation = 'REQUEST_CONFIRMATION',
   removeConfirmationRequest = 'REMOVE_CONFIRMATION_REQUEST',
   toggleAboutDialogVisibility = 'TOGGLE_ABOUT_DIALOG_VISIBILITY',
+  setMobileTab = 'SET_MOBILE_TAB',
 }
 
 export interface ConfirmationRequest {
@@ -27,6 +28,7 @@ export interface GlobalAction extends Action {
   error?: string
   notification?: string
   confirmationRequest?: ConfirmationRequest
+  mobileTab?: number
 }
 
 interface GlobalStateInterface {
@@ -38,6 +40,7 @@ interface GlobalStateInterface {
   settingsVisible: boolean
   confirmationRequests: Array<ConfirmationRequest>
   aboutDialogVisible: boolean
+  mobileTab: number // 0 = topics, 1 = details, 2 = publish, 3 = charts
 }
 
 export type GlobalState = Record<GlobalStateInterface>
@@ -51,6 +54,7 @@ const initialStateFactory = Record<GlobalStateInterface>({
   settingsVisible: false,
   confirmationRequests: [],
   aboutDialogVisible: false,
+  mobileTab: 0,
 })
 
 export const globalState: Reducer<Record<GlobalStateInterface>, GlobalAction> = (
@@ -99,6 +103,12 @@ export const globalState: Reducer<Record<GlobalStateInterface>, GlobalAction> =
         state.get('confirmationRequests').filter(a => a !== action.confirmationRequest)
       )
 
+    case ActionTypes.setMobileTab:
+      if (action.mobileTab === undefined) {
+        return state
+      }
+      return state.set('mobileTab', action.mobileTab)
+
     default:
       return state
   }

src/spec/viewport-switching.spec.ts

@@ -0,0 +1,291 @@
+import 'mocha'
+import { expect } from 'chai'
+import { Browser, BrowserContext, Page, chromium } from 'playwright'
+import { createTestMock, stopTestMock } from './mock-mqtt-test'
+import { sleep } from './util'
+import { connectTo } from './scenarios/connect'
+import type { MqttClient } from 'mqtt'
+
+/**
+ * Viewport Switching Test
+ *
+ * This test checks for React errors when switching between mobile and desktop viewports.
+ * The breakpoint is at 768px width.
+ */
+describe('Viewport Switching Tests', function () {
+  this.timeout(120000)
+
+  let browser: Browser | undefined
+  let browserContext: BrowserContext | undefined
+  let testMock: MqttClient
+  let page: Page
+
+  before(async function () {
+    this.timeout(90000)
+
+    console.log('Creating test-specific MQTT mock...')
+    testMock = await createTestMock()
+
+    console.log('Publishing test topics...')
+    testMock.publish('livingroom/lamp/state', 'on', { retain: true, qos: 0 })
+    testMock.publish('livingroom/lamp/brightness', '128', { retain: true, qos: 0 })
+    testMock.publish('livingroom/temperature', '21.0', { retain: true, qos: 0 })
+
+    const coffeeData = {
+      heater: 'on',
+      temperature: 92.5,
+      waterLevel: 0.5,
+    }
+    testMock.publish('kitchen/coffee_maker', JSON.stringify(coffeeData), { retain: true, qos: 2 })
+    testMock.publish('kitchen/lamp/state', 'off', { retain: true, qos: 0 })
+
+    await sleep(2000) // Let MQTT messages propagate
+
+    console.log('Launching browser...')
+    const browserUrl = process.env.BROWSER_MODE_URL || 'http://localhost:3000'
+    console.log(`Browser URL: ${browserUrl}`)
+
+    browser = await chromium.launch({
+      headless: true,
+      args: ['--no-sandbox', '--disable-dev-shm-usage'],
+    })
+
+    // Start with mobile viewport (below 768px)
+    browserContext = await browser.newContext({
+      viewport: {
+        width: 412,
+        height: 914,
+      },
+      permissions: ['clipboard-read', 'clipboard-write'],
+    })
+    page = await browserContext.newPage()
+
+    // Collect console messages and errors
+    const consoleMessages: string[] = []
+    const pageErrors: Error[] = []
+
+    page.on('console', msg => {
+      const text = msg.text()
+      consoleMessages.push(`[${msg.type()}] ${text}`)
+      console.log('Browser console:', msg.type(), text)
+    })
+    
+    page.on('pageerror', error => {
+      pageErrors.push(error)
+      console.error('Browser error:', error.message)
+    })
+
+    // Store these in page context for access in tests
+    ;(page as any).testConsoleMessages = consoleMessages
+    ;(page as any).testPageErrors = pageErrors
+
+    // Navigate to the browser mode URL
+    await page.goto(browserUrl, { timeout: 30000, waitUntil: 'networkidle' })
+
+    // Handle authentication if required
+    const username = process.env.MQTT_EXPLORER_USERNAME || 'test'
+    const password = process.env.MQTT_EXPLORER_PASSWORD || 'test123'
+
+    console.log('Waiting for page to initialize...')
+    await sleep(5000)
+
+    const loginDialog = page.locator('h2:has-text("Login to MQTT Explorer")')
+    let loginDialogVisible = false
+    try {
+      loginDialogVisible = await loginDialog.isVisible({ timeout: 10000 })
+    } catch (error) {
+      console.log('Login dialog not found - assuming auth is disabled')
+    }
+
+    if (loginDialogVisible) {
+      console.log('Login dialog detected, authenticating...')
+      await page.fill('[data-testid="username-input"] input', username)
+      await page.fill('[data-testid="password-input"] input', password)
+      await page.click('button:has-text("Login")')
+      await sleep(3000)
+      console.log('Authentication complete')
+    }
+
+    // Wait for the connection dialog to appear
+    console.log('Waiting for MQTT connection dialog...')
+    await page.locator('//label[contains(text(), "Host")]/..//input').waitFor({ timeout: 10000 })
+
+    console.log('Connecting to MQTT broker...')
+    const brokerHost = process.env.TESTS_MQTT_BROKER_HOST || '127.0.0.1'
+    await connectTo(brokerHost, page)
+    await sleep(3000) // Give time for topics to load
+    console.log('Setup complete (mobile viewport)')
+  })
+
+  after(async function () {
+    this.timeout(10000)
+
+    if (browserContext) {
+      await browserContext.close()
+    }
+    if (browser) {
+      await browser.close()
+    }
+
+    stopTestMock()
+  })
+
+  describe('Mobile to Desktop Viewport Switch', () => {
+    it('should switch from mobile (412px) to desktop (1280px) without React errors', async function () {
+      // Given: Mobile viewport (412x914) with topics loaded
+      console.log('Current viewport: 412x914 (mobile)')
+      await page.screenshot({ path: 'test-viewport-mobile-before.png', fullPage: true })
+      
+      // Clear previous errors
+      const pageErrors = (page as any).testPageErrors as Error[]
+      const consoleMessages = (page as any).testConsoleMessages as string[]
+      pageErrors.length = 0
+      consoleMessages.length = 0
+
+      // When: Switch to desktop viewport (>768px)
+      console.log('Switching viewport to 1280x720 (desktop)...')
+      await page.setViewportSize({ width: 1280, height: 720 })
+      await sleep(2000) // Wait for resize handlers and re-renders
+
+      console.log('Viewport switched to desktop')
+      await page.screenshot({ path: 'test-viewport-desktop-after.png', fullPage: true })
+
+      // Then: No React errors should occur
+      console.log(`Console messages: ${consoleMessages.length}`)
+      console.log(`Page errors: ${pageErrors.length}`)
+
+      // Filter out common warnings that are not related to the viewport switch
+      const relevantErrors = pageErrors.filter(error => {
+        const message = error.message || error.toString()
+        // Filter out known warnings
+        return !message.includes('IpcRendererEventBus') &&
+               !message.includes('componentWillReceiveProps') &&
+               !message.includes('locale') &&
+               !message.includes('ACE editor')
+      })
+
+      // Check for React-specific errors in console
+      const reactErrors = consoleMessages.filter(msg => 
+        msg.toLowerCase().includes('error') &&
+        (msg.includes('React') || msg.includes('react') || msg.includes('Warning:'))
+      )
+
+      console.log('Relevant page errors:', relevantErrors.length)
+      console.log('React console errors:', reactErrors.length)
+
+      if (relevantErrors.length > 0) {
+        console.error('Page errors detected:')
+        relevantErrors.forEach(err => console.error('  -', err.message))
+      }
+
+      if (reactErrors.length > 0) {
+        console.error('React errors detected:')
+        reactErrors.forEach(msg => console.error('  -', msg))
+      }
+
+      expect(relevantErrors.length, 'Should have no relevant page errors').to.equal(0)
+      expect(reactErrors.length, 'Should have no React console errors').to.equal(0)
+    })
+
+    it('should switch from desktop to mobile without React errors', async function () {
+      // Given: Desktop viewport (1280x720) from previous test
+      console.log('Current viewport: 1280x720 (desktop)')
+      
+      // Clear previous errors
+      const pageErrors = (page as any).testPageErrors as Error[]
+      const consoleMessages = (page as any).testConsoleMessages as string[]
+      pageErrors.length = 0
+      consoleMessages.length = 0
+
+      // When: Switch back to mobile viewport (<768px)
+      console.log('Switching viewport to 412x914 (mobile)...')
+      await page.setViewportSize({ width: 412, height: 914 })
+      await sleep(2000) // Wait for resize handlers and re-renders
+
+      console.log('Viewport switched to mobile')
+      await page.screenshot({ path: 'test-viewport-mobile-after.png', fullPage: true })
+
+      // Then: No React errors should occur
+      const relevantErrors = pageErrors.filter(error => {
+        const message = error.message || error.toString()
+        return !message.includes('IpcRendererEventBus') &&
+               !message.includes('componentWillReceiveProps') &&
+               !message.includes('locale') &&
+               !message.includes('ACE editor')
+      })
+
+      const reactErrors = consoleMessages.filter(msg => 
+        msg.toLowerCase().includes('error') &&
+        (msg.includes('React') || msg.includes('react') || msg.includes('Warning:'))
+      )
+
+      if (relevantErrors.length > 0) {
+        console.error('Page errors detected:')
+        relevantErrors.forEach(err => console.error('  -', err.message))
+      }
+
+      if (reactErrors.length > 0) {
+        console.error('React errors detected:')
+        reactErrors.forEach(msg => console.error('  -', msg))
+      }
+
+      expect(relevantErrors.length, 'Should have no relevant page errors').to.equal(0)
+      expect(reactErrors.length, 'Should have no React console errors').to.equal(0)
+    })
+
+    it('should handle multiple rapid viewport changes', async function () {
+      console.log('Testing rapid viewport changes...')
+      
+      // Clear previous errors
+      const pageErrors = (page as any).testPageErrors as Error[]
+      const consoleMessages = (page as any).testConsoleMessages as string[]
+      pageErrors.length = 0
+      consoleMessages.length = 0
+
+      // Rapidly switch between viewports
+      const viewports = [
+        { width: 600, height: 800, name: 'mobile' },    // < 768
+        { width: 900, height: 600, name: 'desktop' },   // > 768
+        { width: 700, height: 800, name: 'mobile' },    // < 768
+        { width: 1024, height: 768, name: 'desktop' },  // > 768
+        { width: 412, height: 914, name: 'mobile' },    // < 768
+      ]
+
+      for (const vp of viewports) {
+        console.log(`Switching to ${vp.name} (${vp.width}x${vp.height})...`)
+        await page.setViewportSize({ width: vp.width, height: vp.height })
+        await sleep(500) // Short delay between switches
+      }
+
+      await sleep(2000) // Final settle time
+      await page.screenshot({ path: 'test-viewport-rapid-changes.png', fullPage: true })
+
+      // Check for errors
+      const relevantErrors = pageErrors.filter(error => {
+        const message = error.message || error.toString()
+        return !message.includes('IpcRendererEventBus') &&
+               !message.includes('componentWillReceiveProps') &&
+               !message.includes('locale') &&
+               !message.includes('ACE editor')
+      })
+
+      const reactErrors = consoleMessages.filter(msg => 
+        msg.toLowerCase().includes('error') &&
+        (msg.includes('React') || msg.includes('react') || msg.includes('Warning:'))
+      )
+
+      if (relevantErrors.length > 0) {
+        console.error('Page errors detected:')
+        relevantErrors.forEach(err => console.error('  -', err.message))
+      }
+
+      if (reactErrors.length > 0) {
+        console.error('React errors detected:')
+        reactErrors.forEach(msg => console.error('  -', msg))
+      }
+
+      expect(relevantErrors.length, 'Should have no relevant page errors').to.equal(0)
+      expect(reactErrors.length, 'Should have no React console errors').to.equal(0)
+    })
+  })
+})

tsconfig.json

@@ -27,6 +27,7 @@
     "src/spec/testMcpIntrospection.ts",
     "src/spec/ui-tests.spec.ts",
     "src/spec/ui-tests-comprehensive.spec.ts",
+    "src/spec/viewport-switching.spec.ts",
     "src/spec/expandTopic.spec.ts",
     "src/spec/security-tests.spec.ts",
     "src/spec/SceneBuilder.spec.ts",