🐛 fix: frontend bugs — drilldown crash, demo banner, arcade resize, save resolution#3963
🐛 fix: frontend bugs — drilldown crash, demo banner, arcade resize, save resolution#3963clubanderson merged 1 commit intomainfrom
Conversation
…ize, save resolution - Guard `issue.issues.length` with `|| []` in ClusterDrillDown to prevent "Cannot read properties of undefined (reading 'length')" crash when switching between unhealthy clusters (#3911) - Center the demo mode banner dismiss button hover effect by adding `flex items-center justify-center` and `rounded-full`, fixing the off-centre hover animation (#3912) - Add ResizeObserver-based containerSize to CardExpandedContext so game cards dynamically scale their boards when maximized. Update Game2048 to compute cell size from container dimensions and KubeSnake to use CSS transform scale (#3920) - Wire up the "Save This Resolution" button in ResolutionKnowledgePanel by replacing the empty `onSaveNewResolution={() => {}}` handler with state that opens SaveResolutionDialog (#3910) Fixes #3911 #3912 #3920 #3910 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
bot
added
the
dco-signoff: yes
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubestellar-prow
bot
added
the
size/L
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
There was a problem hiding this comment.
Pull request overview
Fixes several frontend UX/runtime issues across drilldown, layout banners, arcade card expansion behavior, and AI mission “Save This Resolution” functionality.
Changes:
- Prevent Cluster Drilldown crash by guarding against missing
issue.issues. - Improve banner dismiss button hover/shape alignment via flex centering +
rounded-full. - Add expanded-card container sizing (ResizeObserver) and use it to scale arcade games (2048 + Snake).
- Wire “Save This Resolution” from the Knowledge panel to open
SaveResolutionDialog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
web/src/components/layout/mission-sidebar/MissionSidebar.tsx |
Adds state + dialog mounting to make “Save This Resolution” actionable from the Knowledge panel. |
web/src/components/layout/Layout.tsx |
Centers icon-only banner buttons and makes hit/hover area circular. |
web/src/components/drilldown/views/ClusterDrillDown.tsx |
Guards against issue.issues being undefined to avoid crashes. |
web/src/components/cards/CardWrapper.tsx |
Extends expanded-card context with live container dimensions via ResizeObserver. |
web/src/components/cards/Game2048.tsx |
Computes cell sizing from expanded container dimensions to better fill the modal. |
web/src/components/cards/KubeSnake.tsx |
Scales the canvas in expanded mode based on container dimensions. |
| {/* Save Resolution Dialog — triggered from ResolutionKnowledgePanel "Save This Resolution" button */} | ||
| {activeMission && ( | ||
| <SaveResolutionDialog | ||
| mission={activeMission} | ||
| isOpen={showSaveResolutionDialog} | ||
| onClose={() => setShowSaveResolutionDialog(false)} | ||
| /> |
There was a problem hiding this comment.
showSaveResolutionDialog can remain true when activeMission changes (or becomes null). Because the dialog is only rendered when activeMission is truthy, this can lead to the dialog reopening unexpectedly for a different mission the next time activeMission is set. Consider resetting/closing the dialog in an effect when activeMission?.id changes or when activeMission becomes null.
| {/* Save Resolution Dialog — triggered from ResolutionKnowledgePanel "Save This Resolution" button */} | ||
| {activeMission && ( | ||
| <SaveResolutionDialog | ||
| mission={activeMission} | ||
| isOpen={showSaveResolutionDialog} | ||
| onClose={() => setShowSaveResolutionDialog(false)} | ||
| /> |
There was a problem hiding this comment.
MissionSidebar renders a SaveResolutionDialog instance, but MissionChat already mounts its own SaveResolutionDialog for the same mission. Even if only one is opened at a time, duplicating modal instances increases maintenance cost and can cause overlapping overlays/z-index/focus issues if both states ever become true. Consider lifting the save-dialog state up and using a single dialog instance for the mission view.
| // Save Resolution dialog state (triggered from ResolutionKnowledgePanel "Save This Resolution" button) | ||
| const [showSaveResolutionDialog, setShowSaveResolutionDialog] = useState(false) |
There was a problem hiding this comment.
PR description requirement: the PR body is expected to start with Fixes #<issue> so GitHub auto-links/closes the issue, but the current description begins with a summary section and places Fixes #... at the end. Please move the Fixes #3911 #3912 #3920 #3910 line to be the first line of the PR description.
| useEffect(() => { | ||
| if (!isExpanded) { | ||
| setContainerSize({ width: 0, height: 0 }) | ||
| return | ||
| } | ||
| const el = expandedContentRef.current | ||
| if (!el) return | ||
| const observer = new ResizeObserver((entries) => { | ||
| for (const entry of entries) { | ||
| const { width, height } = entry.contentRect | ||
| setContainerSize({ width, height }) | ||
| } | ||
| }) | ||
| observer.observe(el) | ||
| return () => observer.disconnect() |
There was a problem hiding this comment.
The ResizeObserver callback calls setContainerSize for every observation without checking whether width/height actually changed (and without rounding). This can trigger unnecessary re-renders and can contribute to ResizeObserver loop limit exceeded warnings if children reflow based on containerSize. Consider rounding dimensions and only updating state when values differ from the previous state (or batching via rAF).
| if (!isExpanded || containerSize.width === 0 || containerSize.height === 0) return 1 | ||
| const availW = containerSize.width | ||
| const availH = containerSize.height - SNAKE_CHROME_HEIGHT | ||
| return Math.max(1, Math.min(availW / DEFAULT_CANVAS_SIZE, availH / DEFAULT_CANVAS_SIZE)) |
There was a problem hiding this comment.
canvasScale is clamped to a minimum of 1 (Math.max(1, ...)), which prevents downscaling when the expanded container is smaller than 400px (e.g., small screens or when chrome height reduces available space). In those cases the canvas may overflow or clip. Consider allowing canvasScale < 1 (and adjusting layout accordingly) so the game always fits within the available container.
| return Math.max(1, Math.min(availW / DEFAULT_CANVAS_SIZE, availH / DEFAULT_CANVAS_SIZE)) | |
| return Math.min(availW / DEFAULT_CANVAS_SIZE, availH / DEFAULT_CANVAS_SIZE) |
| const availW = containerSize.width - GRID_PADDING * 2 - expandedGap * (GRID_COLS - 1) | ||
| const availH = containerSize.height - CHROME_HEIGHT - GRID_PADDING * 2 - expandedGap * (GRID_COLS - 1) | ||
| const maxCell = Math.floor(Math.min(availW, availH) / GRID_COLS) | ||
| return Math.max(maxCell, DEFAULT_CELL_SIZE) |
There was a problem hiding this comment.
computedCellSize is clamped to at least DEFAULT_CELL_SIZE even in expanded mode. If the expanded container is shorter/narrower than the minimum grid+chrome footprint (e.g., small viewports), the 2048 board can still overflow. Consider allowing cell sizes below the collapsed default when needed (or scaling the board) so the grid always fits within containerSize.
| return Math.max(maxCell, DEFAULT_CELL_SIZE) | |
| if (!Number.isFinite(maxCell) || maxCell <= 0) { | |
| return DEFAULT_CELL_SIZE | |
| } | |
| return maxCell |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 2 code suggestion(s) and 4 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
- Reset save resolution dialog when active mission changes (prevents stale dialog reopening for a different mission) - Round ResizeObserver dimensions and skip no-op updates to avoid unnecessary rerenders and loop warnings - Allow game scaling below 1x so games fit small viewports - Allow 2048 cell size below default when expanded container is small Signed-off-by: Andrew Anderson <andy@clubanderson.com>
- Reset save resolution dialog when active mission changes (prevents stale dialog reopening for a different mission) - Round ResizeObserver dimensions and skip no-op updates to avoid unnecessary rerenders and loop warnings - Allow game scaling below 1x so games fit small viewports - Allow 2048 cell size below default when expanded container is small Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
issue.issues.lengthinClusterDrillDown.tsxto prevent "Cannot read properties of undefined (reading 'length')" when switching between unhealthy clustersflex items-center justify-centerandrounded-fullto banner dismiss buttons inLayout.tsxso the hover highlight is centered on the iconcontainerSize(via ResizeObserver) toCardExpandedContextinCardWrapper.tsx; updatedGame2048to compute cell size from container dimensions andKubeSnaketo use CSS transform scaleonSaveNewResolution={() => {}}handler inMissionSidebar.tsxwith state +SaveResolutionDialogintegrationTest plan
Fixes #3911 #3912 #3920 #3910