From c7d28da4ec343e47a5ed59f4ec2190a271d97d09 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 12 Jan 2026 09:16:22 +0100 Subject: [PATCH] Redesign topic details sidebar with clickable navigation and improved mobile layout (WIP - demo video test regression) (#1011) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: thomasnordquist <7721625+thomasnordquist@users.noreply.github.com> Co-authored-by: Thomas Nordquist --- .github/copilot-instructions.md | 109 +++++- app/src/components/ChartPanel/index.tsx | 2 +- app/src/components/Sidebar/DetailsTab.tsx | 327 ++++++++++++++++++ app/src/components/Sidebar/PublishTab.tsx | 53 +++ app/src/components/Sidebar/Sidebar.tsx | 108 ++++-- .../components/Sidebar/SimpleBreadcrumb.tsx | 87 +++++ .../Sidebar/ValueRenderer/ActionButtons.tsx | 17 +- src/spec/inspect-chart-settings.ts | 84 +++++ src/spec/scenarios/copyTopicToClipboard.ts | 6 +- src/spec/scenarios/copyValueToClipboard.ts | 6 +- src/spec/scenarios/saveMessageToFile.ts | 4 +- src/spec/scenarios/showNumericPlot.ts | 19 +- src/spec/ui-tests.spec.ts | 18 +- 13 files changed, 789 insertions(+), 51 deletions(-) create mode 100644 app/src/components/Sidebar/DetailsTab.tsx create mode 100644 app/src/components/Sidebar/PublishTab.tsx create mode 100644 app/src/components/Sidebar/SimpleBreadcrumb.tsx create mode 100644 src/spec/inspect-chart-settings.ts diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0082b52..3de0812 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -20,10 +20,117 @@ - `yarn test:mcp` - Model Context Protocol tests - `yarn test:all` - All tests (unit + demo-video) - `./scripts/runBrowserTests.sh` - Browser mode UI tests (requires mosquitto service) +- `./scripts/uiTests.sh` - Demo video tests with Electron (requires Xvfb, mosquitto) **CI jobs:** `test`, `ui-tests`, `demo-video`, `test-browser`, `browser-ui-tests` -**Important:** Browser UI tests require MQTT broker. In CI, GitHub Actions health checks ensure the mosquitto service is ready before tests run. +**Important:** +- Browser UI tests require MQTT broker. In CI, GitHub Actions health checks ensure the mosquitto service is ready before tests run. +- Demo video tests use the same test scenarios as browser tests - if browser tests pass, demo video tests should pass too (they use identical selectors in `src/spec/scenarios/`) + +## Debugging Demo Video / UI Tests + +When demo video tests fail in CI but you need to debug locally: + +**Prerequisites:** +```bash +# Install required packages +sudo apt-get install xvfb mosquitto tmux ffmpeg + +# Ensure mosquitto is stopped (script will start its own instance) +sudo systemctl stop mosquitto +``` + +**Steps to debug:** +1. Build the project: `yarn build` +2. Run the demo video tests: `ELECTRON_DISABLE_SANDBOX=1 ./scripts/uiTests.sh` +3. If tests fail, check the error messages for: + - Missing `data-test` or `data-test-type` attributes + - Elements not visible (hidden, outside viewport, or covered by overlays) + - Click interception (tooltips, dialogs blocking clicks) + - XPath selector issues (check the data-test value format) + +**Common issues:** +- **"locator not visible"**: Element exists but is hidden or outside viewport +- **"locator.click intercepted"**: Another element (tooltip, dialog) is covering the click target +- **Empty `data-test` attribute**: For simple numeric values, ensure you're using the topic name, not `props.literal.path` (which is empty for non-JSON values) +- **"Process failed to launch"**: Electron can't start - ensure DISPLAY is set and Xvfb is running + +**Environment-specific notes:** +- Demo video tests use Electron with Xvfb (virtual display) +- Browser tests use Chromium without Electron (easier to debug locally) +- CI environment has proper Electron setup - if local tests are flaky, trust CI results +- Both test types use the same scenario files in `src/spec/scenarios/` + +**Material-UI Tooltip considerations:** +- Tooltips wrap their children and create overlay divs +- Test attributes (`data-test-type`, `data-test`) must be on the inner clickable element that's passed as a child to the Tooltip +- Mouse event handlers (onMouseEnter, onMouseLeave, ref) go on outer wrapper +- onClick handler and test attributes go on the span inside the Tooltip +- The clickable child inside the Tooltip is what Playwright should target + +**Example:** +```tsx +// ❌ WRONG - attributes on outer wrapper, Tooltip wraps and hides them + + + + + + +// ✅ CORRECT - attributes on the actual clickable child inside Tooltip + + + + + + + +``` + +**data-test value format:** +- ShowChart: Use last segment of topic path (e.g., "heater" for "kitchen/coffee_maker/heater") +- ChartSettings: Use full topic + dotPath (e.g., "kitchen/coffee_maker/heater-" with trailing dash when dotPath is empty) +- Test XPaths use `contains(@data-test, "substring")` so partial matches work + +## Running Browser Tests Locally + +**Prerequisites:** +```bash +# Install mosquitto (if not already installed) +sudo apt-get install mosquitto + +# Start mosquitto service +sudo systemctl start mosquitto +sudo systemctl status mosquitto +``` + +**Run browser tests:** +```bash +# Build server and run tests +yarn build:server +./scripts/runBrowserTests.sh +``` + +The script automatically: +- Starts a local mosquitto broker +- Builds the TypeScript code +- Starts the browser mode server on port 3000 +- Runs Playwright tests in browser mode +- Cleans up processes on exit + +**Test environment variables:** +- `MQTT_EXPLORER_USERNAME` - Browser auth username (default: test) +- `MQTT_EXPLORER_PASSWORD` - Browser auth password (default: test123) +- `PORT` - Server port (default: 3000) +- `TESTS_MQTT_BROKER_HOST` - MQTT broker host (default: 127.0.0.1) +- `TESTS_MQTT_BROKER_PORT` - MQTT broker port (default: 1883) +- `USE_MOBILE_VIEWPORT` - Enable mobile viewport (default: false) + +**Common test failures after UI changes:** +- Update test selectors in `src/spec/ui-tests.spec.ts` if UI structure changes +- Use `data-testid` attributes for stable test selectors +- Avoid using role + name selectors for dynamic content (use direct testid selectors instead) ## Browser Mode diff --git a/app/src/components/ChartPanel/index.tsx b/app/src/components/ChartPanel/index.tsx index 72a48a3..965a79c 100644 --- a/app/src/components/ChartPanel/index.tsx +++ b/app/src/components/ChartPanel/index.tsx @@ -154,7 +154,7 @@ const styles = (theme: Theme) => ({ height: '100%', padding: '8px', flex: 1, - overflow: 'hidden scroll', + overflow: 'auto', }, }) diff --git a/app/src/components/Sidebar/DetailsTab.tsx b/app/src/components/Sidebar/DetailsTab.tsx new file mode 100644 index 0000000..3233731 --- /dev/null +++ b/app/src/components/Sidebar/DetailsTab.tsx @@ -0,0 +1,327 @@ +import * as q from '../../../../backend/src/Model' +import React, { useCallback } from 'react' +import { Box, Typography, IconButton, Chip, Tooltip } from '@mui/material' +import { Theme } from '@mui/material/styles' +import { withStyles } from '@mui/styles' +import { AppState } from '../../reducers' +import { connect } from 'react-redux' +import { bindActionCreators } from 'redux' +import { sidebarActions } from '../../actions' +import Copy from '../helper/Copy' +import Save from '../helper/Save' +import DateFormatter from '../helper/DateFormatter' +import ValueRenderer from './ValueRenderer/ValueRenderer' +import MessageHistory from './ValueRenderer/MessageHistory' +import ActionButtons from './ValueRenderer/ActionButtons' +import DeleteSelectedTopicButton from './ValueRenderer/DeleteSelectedTopicButton' +import { useDecoder } from '../hooks/useDecoder' +import DeleteIcon from '@mui/icons-material/Delete' +import DeleteSweepIcon from '@mui/icons-material/DeleteSweep' +import SimpleBreadcrumb from './SimpleBreadcrumb' + +interface Props { + node?: q.TreeNode + classes: any + compareMessage?: q.Message + sidebarActions: typeof sidebarActions +} + +function DetailsTab(props: Props) { + const { node, compareMessage, classes } = props + const decodeMessage = useDecoder(node) + + const getDecodedValue = useCallback(() => { + return node?.message && decodeMessage(node.message)?.message?.toUnicodeString() + }, [node, decodeMessage]) + + const getData = () => { + if (node?.message && node.message.payload) { + return node.message.payload.base64Message + } + } + + const handleMessageHistorySelect = useCallback( + (message: q.Message) => { + if (message !== compareMessage) { + props.sidebarActions.setCompareMessage(message) + } else { + props.sidebarActions.setCompareMessage(undefined) + } + }, + [compareMessage, props.sidebarActions] + ) + + const deleteTopic = useCallback( + (topic?: q.TreeNode, recursive: boolean = false) => { + if (!topic) { + return + } + props.sidebarActions.clearTopic(topic, recursive) + }, + [props.sidebarActions] + ) + + if (!node) { + return ( + + + Select a topic to view details + + + ) + } + + const [value] = + node && node.message && node.message.payload ? node.message.payload?.format(node.type) : [null, undefined] + const hasValue = Boolean(value) + + return ( + + {/* Topic Section - Breadcrumb with actions */} + + + + + {node.childTopicCount() === 0 && ( + + deleteTopic(node, false)} className={classes.iconButton}> + + + + )} + {node.childTopicCount() > 0 && ( + + deleteTopic(node, true)} className={classes.iconButton}> + + + + )} + + + + {/* Value Section - Simplified layout */} + {hasValue && ( + + {/* Metadata bar - Date on left, Retained/QoS on right */} + + + + + + + + {node.message?.retain && ( + + )} + + + + + {/* Action toolbar */} + + + Current Value + + + + + + + + {node.message?.retain && } + + + + {/* Value Display */} + + Loading...}> + + + + + {/* Message History */} + + + + + {/* Stats Section - Moved to end of value section */} + + + + + Messages + + + {node.messages} + + + + + Subtopics + + + {node.childTopicCount()} + + + + + Total + + + {node.leafMessageCount()} + + + + + + )} + + ) +} + +const styles = (theme: Theme) => ({ + root: { + display: 'flex', + flexDirection: 'column' as 'column', + gap: theme.spacing(3), + [theme.breakpoints.down('sm')]: { + gap: theme.spacing(2), + }, + }, + emptyState: { + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + minHeight: '200px', + padding: theme.spacing(3), + }, + // Topic section + topicSection: { + display: 'flex', + justifyContent: 'space-between', + alignItems: 'flex-start', + gap: theme.spacing(1), + paddingBottom: theme.spacing(2), + borderBottom: `1px solid ${theme.palette.divider}`, + }, + topicActions: { + display: 'flex', + gap: theme.spacing(0.5), + alignItems: 'center', + flexShrink: 0, + }, + iconButton: { + padding: theme.spacing(0.5), + }, + // Stats section + statsSection: { + marginTop: theme.spacing(2), + }, + statsGrid: { + display: 'grid', + gridTemplateColumns: 'repeat(3, 1fr)', + gap: theme.spacing(1.5), + [theme.breakpoints.down('sm')]: { + gap: theme.spacing(1), + }, + }, + statItem: { + display: 'flex', + flexDirection: 'column' as 'column', + alignItems: 'center', + padding: theme.spacing(1.5, 1), + backgroundColor: theme.palette.action.hover, + borderRadius: theme.shape.borderRadius, + gap: theme.spacing(0.5), + }, + statLabel: { + fontSize: '0.75rem', + fontWeight: 500, + textTransform: 'uppercase' as 'uppercase', + letterSpacing: '0.5px', + }, + statValue: { + fontSize: '1.25rem', + fontWeight: 600, + lineHeight: 1, + }, + // Value section + valueSection: { + display: 'flex', + flexDirection: 'column' as 'column', + gap: theme.spacing(2), + }, + metadataBar: { + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + gap: theme.spacing(1), + flexWrap: 'wrap' as 'wrap', + padding: theme.spacing(1), + backgroundColor: theme.palette.action.hover, + borderRadius: theme.shape.borderRadius, + }, + metadataLeft: { + display: 'flex', + gap: theme.spacing(1), + alignItems: 'center', + flexWrap: 'wrap' as 'wrap', + }, + metadataRight: { + display: 'flex', + alignItems: 'center', + }, + chip: { + height: '24px', + }, + actionToolbar: { + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + gap: theme.spacing(1), + flexWrap: 'wrap' as 'wrap', + }, + valueTitle: { + fontWeight: 600, + color: theme.palette.text.primary, + fontSize: '0.875rem', + textTransform: 'uppercase' as 'uppercase', + letterSpacing: '0.5px', + flexShrink: 0, + }, + actionButtons: { + display: 'flex', + alignItems: 'center', + flex: 1, + }, + valueActions: { + display: 'flex', + gap: theme.spacing(0.5), + alignItems: 'center', + }, + valueDisplay: { + marginTop: theme.spacing(1), + }, + historySection: { + marginTop: theme.spacing(1), + }, +}) + +const mapStateToProps = (state: AppState) => { + return { + compareMessage: state.sidebar.get('compareMessage'), + } +} + +const mapDispatchToProps = (dispatch: any) => { + return { + sidebarActions: bindActionCreators(sidebarActions, dispatch), + } +} + +export default connect(mapStateToProps, mapDispatchToProps)(withStyles(styles)(DetailsTab)) diff --git a/app/src/components/Sidebar/PublishTab.tsx b/app/src/components/Sidebar/PublishTab.tsx new file mode 100644 index 0000000..8a3e044 --- /dev/null +++ b/app/src/components/Sidebar/PublishTab.tsx @@ -0,0 +1,53 @@ +import React from 'react' +import { Box, Typography } from '@mui/material' +import { Theme } from '@mui/material/styles' +import { withStyles } from '@mui/styles' + +const Publish = React.lazy(() => import('./Publish/Publish')) + +interface Props { + connectionId?: string + classes: any +} + +function PublishTab(props: Props) { + const { classes } = props + + return ( + + + + Publish Message + + + Send messages to MQTT topics + + + + Loading...}> + + + + ) +} + +const styles = (theme: Theme) => ({ + root: { + display: 'flex', + flexDirection: 'column' as 'column', + gap: theme.spacing(2), + }, + header: { + marginBottom: theme.spacing(1), + }, + title: { + fontWeight: 600, + color: theme.palette.text.primary, + fontSize: '0.875rem', + textTransform: 'uppercase' as 'uppercase', + letterSpacing: '0.5px', + marginBottom: theme.spacing(0.5), + }, +}) + +export default withStyles(styles)(PublishTab) diff --git a/app/src/components/Sidebar/Sidebar.tsx b/app/src/components/Sidebar/Sidebar.tsx index 963ade1..4675069 100644 --- a/app/src/components/Sidebar/Sidebar.tsx +++ b/app/src/components/Sidebar/Sidebar.tsx @@ -1,24 +1,19 @@ import * as q from '../../../../backend/src/Model' import React, { useState, useEffect, useCallback } from 'react' -import NodeStats from './NodeStats' -import ValuePanel from './ValueRenderer/ValuePanel' -const ValuePanelAny = ValuePanel as any import { AppState } from '../../reducers' -import { AccordionDetails } from '@mui/material' import { bindActionCreators } from 'redux' import { connect } from 'react-redux' import { settingsActions, sidebarActions } from '../../actions' import { Theme } from '@mui/material/styles' import { withStyles } from '@mui/styles' import { TopicViewModel } from '../../model/TopicViewModel' -import TopicPanel from './TopicPanel/TopicPanel' -import Panel from './Panel' import { usePollingToFetchTreeNode } from '../helper/usePollingToFetchTreeNode' +import { Tabs, Tab, Box, useMediaQuery, useTheme } from '@mui/material' +import DetailsTab from './DetailsTab' +import PublishTab from './PublishTab' const throttle = require('lodash.throttle') -const Publish = React.lazy(() => import('./Publish/Publish')) - interface Props { nodePath?: string tree?: q.Tree @@ -49,27 +44,55 @@ function useUpdateNodeWhenNodeReceivesUpdates(node?: q.TreeNode) { }, [node]) } -function Sidebar(props: Props) { +function SidebarNew(props: Props) { const { classes, tree, nodePath } = props const node = usePollingToFetchTreeNode(tree, nodePath || '') useUpdateNodeWhenNodeReceivesUpdates(node) + const [tabValue, setTabValue] = useState(0) + const theme = useTheme() + const isMobile = useMediaQuery(theme.breakpoints.down('sm')) - return ( -