Skip to content

feat: Collapsible sections — authoring UX + DashboardContainer abstraction#1926

Open
alex-fedotyev wants to merge 8 commits intomainfrom
feat/collapsible-sections-authoring-ux
Open

feat: Collapsible sections — authoring UX + DashboardContainer abstraction#1926
alex-fedotyev wants to merge 8 commits intomainfrom
feat/collapsible-sections-authoring-ux

Conversation

@alex-fedotyev
Copy link
Contributor

@alex-fedotyev alex-fedotyev commented Mar 17, 2026

Summary

Adds the authoring experience for dashboard sections (create, rename, delete, manage tiles) and introduces a polymorphic DashboardContainer abstraction that future-proofs the schema for tabs and groups.

Builds on #1900 (core collapsible sections mechanics). Closes #1897.

Schema: DashboardSectionDashboardContainer

  • Renamed DashboardSectionSchemaDashboardContainerSchema with a new type field ('section' for now, extensible to 'group' / 'tab' later)
  • sectionIdcontainerId on tiles
  • sectionscontainers on dashboards
  • Updated across all packages: common-utils types, API Mongoose model, app types, import/export utils

Authoring UX

Action How
Create section Dashboard ... overflow menu → "Add Section"
Rename section Click the title text directly (Kibana-style inline editing)
Delete section Hover section header → ... → Delete Section (tiles become ungrouped, not deleted)
Collapse/expand Click section header chevron
Toggle default state Hover header → ... → Collapse/Expand by Default
Add tile to section Hover section header → + button opens tile editor pre-assigned to that section
Move tile to section Hover tile → grid icon → pick target section from dropdown
Move tile out Same dropdown → "(Ungrouped)"

UX polish (informed by Grafana/Datadog/Kibana competitor research)

  • Click-to-rename — click section title text to edit inline (no menu navigation needed)
  • Hover-only controls... menu and + button only appear on section header hover, keeping view mode clean
  • "Add Section" demoted — moved from equal-sized button to dashboard overflow menu (section creation is less frequent than tile creation)
  • "Move to Section" reordered — placed before delete button for discoverability, uses IconLayoutList instead of IconFolders

What's NOT in this PR (follow-up work)

  • Drag tiles between sections — needs react-dnd custom drag layer; data model already supports it (containerId update)
  • Reorder sections — needs sortable list library; data model supports it (array order)
  • Tabs / Groups — new container types; just add to the type enum and build UIs

Test plan

  • 30 unit tests pass (16 existing schema/grouping + 14 new authoring operations)
  • All 110 dashboard tests pass unchanged
  • ESLint clean
  • No TypeScript errors in changed files
  • Backward compatible — dashboards without containers render exactly as before

🤖 Generated with Claude Code

Add section management to the dashboard: create, rename, delete sections,
toggle default collapsed state, and move tiles between sections via a
menu on the tile hover toolbar.

Closes #1897
@vercel
Copy link

vercel bot commented Mar 17, 2026

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

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Mar 18, 2026 1:37am

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2026

⚠️ No Changeset found

Latest commit: ec3a8a2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2026

PR Review

  • Breaking schema rename without migration: sectionscontainers and sectionIdcontainerId are renamed in the MongoDB model and Zod schemas, but there is no database migration script. Any production dashboards already saved with sections/sectionId data will silently lose their section configuration on deployment — those fields won't be read by the new containers/containerId lookups. Either add a migration (e.g., db.dashboards.updateMany({sections: {$exists: true}}, [{$set: {containers: '$sections'}}, {$unset: 'sections'}]) plus tile-level sectionIdcontainerId) or make the schema read both field names for backward compatibility.

  • ⚠️ Unit tests test re-implemented logic, not actual handlers: The new describe('section authoring operations') block defines its own local deleteSection, moveTileToSection, etc. functions that reimport/reimplment the production logic as pure helpers, rather than testing the actual handleDeleteSection/handleMoveTileToSection callbacks in DBDashboardPage. If the production implementations diverge from these helpers, tests won't catch it. Consider testing the actual callbacks (e.g., via a render test or extracting the logic to a separately testable module).

✅ Otherwise: renaming is complete and consistent across all packages, SectionHeader rewrite follows project patterns (Mantine components, data-testid attrs, proper useCallback/useMemo usage), hover-only controls match UX conventions, and the handleDeleteSection y-offset logic correctly avoids tile collisions with ungrouped tiles.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2026

E2E Test Results

All tests passed • 92 passed • 3 skipped • 945s

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

Tests ran across 4 shards in parallel.

View full report →

Schema changes:
- DashboardSection → DashboardContainer with `type` field (extensible
  to 'group'/'tab' later, only 'section' for now)
- sectionId → containerId on tiles
- sections → containers on dashboard

UX improvements:
- Click section title to rename directly (Kibana pattern)
- Edit controls (... menu) only visible on hover
- "Add Section" moved to dashboard overflow menu (not equal to Add Tile)
- "Move to Section" icon reordered before delete for discoverability
- IconLayoutList replaces IconFolders for clearer visual meaning
- "+" button on section header to add tile directly into that section
@alex-fedotyev alex-fedotyev changed the title feat: Collapsible sections - authoring UX feat: Collapsible sections — authoring UX + DashboardContainer abstraction Mar 17, 2026
@alex-fedotyev
Copy link
Contributor Author

@claude - code review

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2026

Claude finished @alex-fedotyev's task in 2m 17s —— View job


Code Review

  • Gather context and understand request
  • Read changed files
  • Review API model changes
  • Review common-utils type changes
  • Review frontend components
  • Review tests

Overall this is well-structured work. The schema abstraction is clean, the UX is thoughtful, and the tests are thorough. A few issues to address before merging.

🔴 Critical

Data migration missing — existing sections data will be silently dropped

PR #1900 introduced sections to MongoDB. This PR renames to containers with no migration. Any dashboards saved after #1900 are stored as { sections: [...] } in MongoDB. After this deploy, those documents still have sections in the DB but the app now reads dashboard.containers, which will be undefined — users lose their sections silently.

Fix with a read-time fallback in the API layer or a one-time migration:

// packages/api/src/models/dashboard.ts or the dashboard controller
containers: { type: mongoose.Schema.Types.Array, required: false },
sections: { type: mongoose.Schema.Types.Array, required: false }, // keep for migration

Or a read-time normalizer in the API response handler:

// When returning a dashboard, normalize legacy field
if (!dashboard.containers && (dashboard as any).sections) {
  dashboard.containers = (dashboard as any).sections;
}

Fix this →


type field is required but breaks Zod parse for containers saved via #1900

DashboardContainerSchema now requires type: z.enum(['section']). Any container documents stored by the previous PR (which didn't have a type field) will fail Zod validation. This is separate from the field rename — it would affect even if the field was still called sections.

Consider making it optional with a default: type: z.enum(['section']).default('section'), or at minimum add a .transform() to fill it in on read.

Fix this →


🟡 Warnings

onToggle and onToggleDefaultCollapsed are functionally identical (packages/app/src/DBDashboardPage.tsx:1677–1684)

Both call handleToggleSection(section.id) which just flips collapsed. There's no semantic difference between "click chevron" and "Collapse by Default" — both toggle the same persisted collapsed field. The prop name onToggleDefaultCollapsed implies it sets a default in some distinct way, but it doesn't. Either:

  • Document this explicitly in a code comment
  • Or use a single onToggle prop for both (remove onToggleDefaultCollapsed from SectionHeader)

The menu label already communicates intent ("Collapse by Default" / "Expand by Default"), so the UX isn't broken — just the prop API is misleading.


inputRef is declared and attached but never used (packages/app/src/components/SectionHeader.tsx:33)

const inputRef = useRef<HTMLInputElement>(null); // line 33
// ...
<Input ref={inputRef} autoFocus ... /> // line 107

autoFocus already handles focusing on mount. inputRef is never called (no inputRef.current?.focus() or similar). Either remove it, or if it's intended for future use like inputRef.current?.select(), add that call now.


Accessibility: outer <Flex> has cursor: 'pointer' but is non-interactive (packages/app/src/components/SectionHeader.tsx:63)

The outer container sets cursor: 'pointer' via style={{ cursor: 'pointer', ... }}, but role="button", tabIndex, onClick, and onKeyDown are all on the inner <Flex>. Hovering over the padding area or the action icons in the outer container shows a pointer cursor, implying clickability where there is none. Change the outer style to cursor: 'default' and let the inner role="button" element use cursor: 'pointer'.


🔵 Minor / Suggestions

No confirmation on "Delete Section" (packages/app/src/DBDashboardPage.tsx:1681)

useConfirm is already used for tile deletion (line 1115) and saving flows, but section delete fires immediately. Given the destructive nature (removing the container, affecting potentially many tiles), a confirm prompt would match the existing delete patterns.


"New Section" title always hardcoded (packages/app/src/DBDashboardPage.tsx:1243)

Multiple rapid "Add Section" calls produce multiple containers all titled "New Section". Since IDs are unique this isn't a functional bug, but a minor quality-of-life improvement would be to auto-increment: New Section ${sections.length + 1}.


"Move to Section" menu appears even if only one section exists and tile is in it

availableSections.length > 0 shows the menu, but if sections.length === 1 and chart.containerId === sections[0].id, the dropdown only contains "(Ungrouped)". That's correct UX-wise, but worth a comment to explain why the single-section case is still useful.


✅ What's good

  • Schema design is solid — type enum for future discriminated union is exactly right
  • tilesByContainerId orphan handling (line 1297: !tilesByContainerId.has(t.containerId)) prevents tiles from being silently hidden if a section is deleted mid-session
  • Hover-only controls keep view mode clean
  • 14 new unit tests covering all authoring operations — good coverage
  • convertToDashboard{Template,Document} in utils.ts correctly propagates containers field to import/export

@alex-fedotyev
Copy link
Contributor Author

Addressing code review feedback

Fixed (6a45d25)

  • Removed unused inputRefautoFocus on <Input> already handles focus, ref was never called
  • Fixed accessibility: cursor: 'pointer' moved to inner interactive <Flex> — outer container no longer implies clickability on non-interactive areas (padding, action icons)
  • Removed redundant onToggleDefaultCollapsed prop — was identical to onToggle (both called handleToggleSection). The "Collapse/Expand by Default" menu item now calls onToggle directly. The menu label communicates the intent; a separate prop was misleading.

Deferred

Not changing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard: Collapsible sections - authoring UX

1 participant