Skip to content

feat(dashboard): move tile fullscreen to a top-level toolbar icon - #2720

Merged
kodiakhq[bot] merged 2 commits into
mainfrom
alex/dashboard-fullscreen-toolbar-icon
Jul 28, 2026
Merged

feat(dashboard): move tile fullscreen to a top-level toolbar icon#2720
kodiakhq[bot] merged 2 commits into
mainfrom
alex/dashboard-fullscreen-toolbar-icon

Conversation

@alex-fedotyev

Copy link
Copy Markdown
Contributor

Move the dashboard tile's fullscreen action out of the "More actions" menu and into a dedicated top-level toolbar icon, so viewing a tile fullscreen is one click instead of two.

Summary

Fullscreen is a primary action, so it now renders as an IconArrowsMaximize button directly in the tile toolbar next to the alert bell, instead of a Menu.Item in the kebab. Narrow tiles that collapse the whole toolbar into the menu keep the action there. The f keyboard shortcut and the tile-fullscreen-button test id are unchanged, so the existing e2e keeps working.

Test plan

  • eslint + tsc clean
  • manual: hover a tile; the fullscreen icon shows in the toolbar and opens the fullscreen view; f still toggles it
  • narrow tile: the toolbar collapses and fullscreen stays in the menu
  • light and dark themes

[ui-states: allow] Toolbar action button; no empty/loading/error states of its own.

Screenshots

toolbar, light

toolbar, dark

View fullscreen was a Menu.Item inside the tile's "More actions" kebab.
Promote it to a dedicated IconArrowsMaximize button in the toolbar next to
the alert bell, so it is one click instead of two. Narrow tiles that
collapse the toolbar keep fullscreen in the menu. The `f` shortcut and the
tile-fullscreen-button test id are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jul 28, 2026 2:03pm
hyperdx-storybook Ready Ready Preview, Comment Jul 28, 2026 2:03pm

Request Review

@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 063c685

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jul 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 1
  • Production lines changed: 21
  • Branch: alex/dashboard-fullscreen-toolbar-icon
  • Author: alex-fedotyev

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR promotes the dashboard tile fullscreen action from the overflow menu to a dedicated toolbar icon while preserving the existing callback, test identifier, keyboard shortcut, and narrow-tile menu behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain.

Important Files Changed

Filename Overview
packages/app/src/DBDashboardPage.tsx Moves the existing fullscreen action into the primary tile toolbar without changing its behavior.
.changeset/tile-fullscreen-toolbar-icon.md Adds an app patch changeset documenting the fullscreen toolbar update.

Reviews (2): Last reviewed commit: "Merge branch 'main' into alex/dashboard-..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 242 passed • 1 skipped • 1066s

Status Count
✅ Passed 242
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The relocation itself is sound — the new ActionIcon sits inside the existing onMouseDown stopPropagation guard (DBDashboardPage.tsx:701) so react-grid-layout drag suppression is inherited, suffixCount={1} remains correct because hoverToolbar is still a single toolbarItems entry, and the collapsed toolbar is gated by display: none rather than opacity so the hidden copy leaves the tab order and hit-testing entirely.

🟡 P2 -- recommended

  • packages/app/tests/e2e/page-objects/DashboardPage.ts:1357 -- openFullscreenForTile still calls openTileActionsMenu(index) before locating the fullscreen button, so the only e2e path to fullscreen opens the kebab dropdown that the button no longer lives in, and its unscoped [data-testid^="tile-fullscreen-button-"].first() now matches an always-mounted per-tile button rather than the single dropdown-scoped node it resolved to previously.
    • Fix: Drop the openTileActionsMenu call, scope the locator with .nth(index), and add an assertion that the toolbar icon is clickable while the kebab is closed plus a negative assertion that no fullscreen item remains in the kebab dropdown.
    • testing, kieran-typescript, correctness
  • packages/app/src/DBDashboardPage.tsx:459 -- openFullscreen unconditionally reseeds fullscreenDateRange, fullscreenInputValue, and fullscreenGranularity from the dashboard, and the trigger is now a permanently mounted button reachable by keyboard behind its own open modal because FullscreenPanelModal sets trapFocus={false}, so re-activating it discards in-modal time-range and granularity edits.
    • Fix: Return early from openFullscreen when isFullscreen is already true, adding isFullscreen to the useCallback dependency list.
    • julik-frontend-races
  • packages/app/src/DBDashboardPage.tsx:744 -- hoverToolbar and collapsedMenuItems are two hand-maintained copies of the same six tile actions, including re-derived alert-support conditions at :690 and :878, and this change updated only the wide-tile copy so narrow tiles now render the label View fullscreen while wide tiles render View fullscreen (f).
    • Fix: Sync the two labels immediately, then build both surfaces from one ordered array of action descriptors so a single edit updates the wide toolbar and the collapsed list together.
    • maintainability, kieran-typescript
🔵 P3 nitpicks (4)
  • packages/app/src/DBDashboardPage.tsx:749 -- onClick={() => openFullscreen()} wraps a zero-argument useCallback in a throwaway closure while every sibling handler in the same toolbar is passed by bare reference.
    • Fix: Use onClick={openFullscreen} at both :749 and :908.
  • packages/app/src/DBDashboardPage.tsx:744 -- The tooltip advertises the f shortcut, but the hotkey guard at :493 reads isFocused, which is set only by onMouseOver/onMouseLeave at :1308-1315, so the shortcut is mouse-hover-scoped rather than focus-scoped.
    • Fix: Set isFocused from onFocus/onBlur in addition to the pointer handlers so the advertised shortcut works once a user has learned it without hovering.
  • packages/app/tests/e2e/page-objects/DashboardPage.ts:601 -- The openTileActionsMenu docstring still states that the kebab reveals the fullscreen item, which no longer matches the dropdown contents at DBDashboardPage.tsx:768-852.
    • Fix: Rewrite the docstring to list only Duplicate, Edit, Move to Group, and Delete.
  • packages/app/src/DBDashboardPage.tsx:906 -- The collapsed-toolbar fullscreen Menu.Item carries no data-testid, so the narrow-tile fallback has no locator even though the wide-tile equivalent at :746 does.
    • Fix: Add data-testid={tile-fullscreen-menu-item-${chart.id}} to the collapsed Menu.Item.

Reviewers (8): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, agent-native, learnings-researcher

Testing gaps:

  • The single e2e touching this feature (dashboard.spec.ts:1610) reaches fullscreen through the kebab, so nothing asserts the one-click toolbar path this change ships; a revert that re-nests the action would pass.
  • No assertion that the fullscreen entry is absent from the kebab dropdown, so a future re-add would silently produce two fullscreen surfaces on wide tiles.
  • The narrow-tile collapsed fullscreen branch is unreachable to the suite — no data-testid, no page-object locator, and jest.config.js:35-53 exempts React components from coverage.
  • No coverage for pressing f to close after opening via the new button, the path where isFocused has flipped false and only FullscreenPanelModal.tsx:19 keeps the shortcut alive.

Coverage notes: Bash and WebFetch were both unavailable in this environment (bwrap: Can't create file at /home/.mcp.json), so git diff could not be run; the change surface was reconstructed by reading HEAD and cross-checking against the changeset, and reviewers worked from Read/Grep/Glob only. Two low-confidence findings were suppressed (shift-click on the new icon also toggling tile selection; collapsed-menu action ordering), and three pre-existing items unchanged by this diff were excluded (the unvalidated as Granularity cast at :457/:465, duplicate per-tile f listeners, and DBDashboardPage.tsx exceeding the 300-line guideline in agent_docs/code_style.md:17). Separately, the untracked ce-plugin/ directory in the working tree is not matched by any .gitignore rule — out of review scope, but worth ignoring before it is committed accidentally. Standards audit found no violations: variant="subtle" and the icon-only ActionIcon choice match agent_docs/code_style.md:36-46,80, the changeset satisfies AGENTS.md:205-211, and no aria-label rule exists for icon buttons in this repo. No agent-native parity gap — fullscreen is ephemeral client state with no representation in the dashboard document or the MCP tool surface. No docs/solutions/ learnings archive exists in this repo.

@pulpdrew pulpdrew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kodiakhq
kodiakhq Bot merged commit ec161d7 into main Jul 28, 2026
27 checks passed
@kodiakhq
kodiakhq Bot deleted the alex/dashboard-fullscreen-toolbar-icon branch July 28, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants