-
Notifications
You must be signed in to change notification settings - Fork 390
refactor: Unify section/group into single Group with collapsible/bordered options #2015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
69a337a
33ea2bb
cc4d62f
7742b48
bcce771
c244076
b018011
e501956
09831a0
dbec2c1
649bea4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@hyperdx/app": patch | ||
| "@hyperdx/common-utils": patch | ||
| --- | ||
|
|
||
| refactor: Unify section/group into single Group with collapsible/bordered options |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| import React from 'react'; | ||
| import { useSortable } from '@dnd-kit/sortable'; | ||
| import { CSS } from '@dnd-kit/utilities'; | ||
| import { Box, Button } from '@mantine/core'; | ||
| import { IconPlus } from '@tabler/icons-react'; | ||
|
|
||
| import { type DragData, type DragHandleProps } from './DashboardDndContext'; | ||
|
|
||
| // --- Empty container placeholder --- | ||
| // Visual placeholder for empty groups/tabs with optional add-tile click. | ||
|
|
||
| export function EmptyContainerPlaceholder({ | ||
| containerId, | ||
| children, | ||
| isEmpty, | ||
| onAddTile, | ||
| }: { | ||
| containerId: string; | ||
| children?: React.ReactNode; | ||
| isEmpty?: boolean; | ||
| onAddTile?: () => void; | ||
| }) { | ||
| return ( | ||
| <div | ||
| data-testid={`container-placeholder-${containerId}`} | ||
| style={{ | ||
| minHeight: isEmpty ? 80 : undefined, | ||
| borderRadius: 4, | ||
| border: isEmpty | ||
| ? '2px dashed var(--mantine-color-default-border)' | ||
| : undefined, | ||
| position: 'relative', | ||
| }} | ||
| > | ||
|
alex-fedotyev marked this conversation as resolved.
Outdated
|
||
| {isEmpty && ( | ||
| <Box | ||
| style={{ | ||
| position: 'absolute', | ||
| inset: 0, | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| padding: '0 16px', | ||
| }} | ||
|
alex-fedotyev marked this conversation as resolved.
Outdated
|
||
| > | ||
| <Button | ||
| variant="secondary" | ||
| fw={400} | ||
| w="100%" | ||
| leftSection={<IconPlus size={16} />} | ||
| onClick={onAddTile} | ||
| > | ||
| Add | ||
| </Button> | ||
| </Box> | ||
| )} | ||
| {children} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // --- Sortable container wrapper (for container reordering) --- | ||
|
|
||
| export function SortableContainerWrapper({ | ||
| containerId, | ||
| containerTitle, | ||
| children, | ||
| }: { | ||
| containerId: string; | ||
| containerTitle: string; | ||
| children: (dragHandleProps: DragHandleProps) => React.ReactNode; | ||
| }) { | ||
| const { | ||
| attributes, | ||
| listeners, | ||
| setNodeRef, | ||
| transform, | ||
| transition, | ||
| isDragging, | ||
| } = useSortable({ | ||
| id: `container-sort-${containerId}`, | ||
| data: { | ||
| type: 'container', | ||
| containerId, | ||
| containerTitle, | ||
| } satisfies DragData, | ||
| }); | ||
|
|
||
| const style: React.CSSProperties = { | ||
| transform: CSS.Transform.toString(transform), | ||
| transition, | ||
| opacity: isDragging ? 0.5 : 1, | ||
| }; | ||
|
|
||
| return ( | ||
| <div ref={setNodeRef} style={style}> | ||
| {children({ ...attributes, ...listeners })} | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| import React, { useCallback, useMemo, useState } from 'react'; | ||
| import { | ||
| DndContext, | ||
| DragEndEvent, | ||
| DragOverlay, | ||
| DragStartEvent, | ||
| MouseSensor, | ||
| TouchSensor, | ||
| useSensor, | ||
| useSensors, | ||
| } from '@dnd-kit/core'; | ||
| import { | ||
| SortableContext, | ||
| verticalListSortingStrategy, | ||
| } from '@dnd-kit/sortable'; | ||
| import { DashboardContainer } from '@hyperdx/common-utils/dist/types'; | ||
| import { Box, Text } from '@mantine/core'; | ||
|
|
||
| // --- Types --- | ||
|
|
||
| export type DragHandleProps = React.HTMLAttributes<HTMLElement>; | ||
|
|
||
| export type DragData = { | ||
| type: 'container'; | ||
| containerId: string; | ||
| containerTitle: string; | ||
| }; | ||
|
|
||
| type Props = { | ||
| children: React.ReactNode; | ||
| containers: DashboardContainer[]; | ||
| onReorderContainers: (fromIndex: number, toIndex: number) => void; | ||
| }; | ||
|
|
||
| // --- Provider (container reorder only) --- | ||
|
|
||
| export function DashboardDndProvider({ | ||
| children, | ||
| containers, | ||
| onReorderContainers, | ||
| }: Props) { | ||
| const [activeDrag, setActiveDrag] = useState<DragData | null>(null); | ||
|
|
||
| const mouseSensor = useSensor(MouseSensor, { | ||
| activationConstraint: { distance: 8 }, | ||
| }); | ||
| const touchSensor = useSensor(TouchSensor, { | ||
| activationConstraint: { delay: 200, tolerance: 5 }, | ||
| }); | ||
| const sensors = useSensors(mouseSensor, touchSensor); | ||
|
|
||
| const containerSortableIds = useMemo( | ||
| () => containers.map(c => `container-sort-${c.id}`), | ||
| [containers], | ||
| ); | ||
|
|
||
| const handleDragStart = useCallback((event: DragStartEvent) => { | ||
| setActiveDrag((event.active.data.current as DragData) ?? null); | ||
| }, []); | ||
|
|
||
| const handleDragEnd = useCallback( | ||
| (event: DragEndEvent) => { | ||
| const { active, over } = event; | ||
| setActiveDrag(null); | ||
| if (!over) return; | ||
|
|
||
| const activeData = active.data.current as DragData | undefined; | ||
| if (!activeData) return; | ||
|
|
||
| // Container reorder via sortable | ||
| const overData = over.data.current as DragData | undefined; | ||
| if ( | ||
| overData?.type === 'container' && | ||
| activeData.containerId !== overData.containerId | ||
| ) { | ||
| const from = containers.findIndex(c => c.id === activeData.containerId); | ||
| const to = containers.findIndex(c => c.id === overData.containerId); | ||
| if (from !== -1 && to !== -1) onReorderContainers(from, to); | ||
| } | ||
| }, | ||
| [containers, onReorderContainers], | ||
| ); | ||
|
|
||
| return ( | ||
| <DndContext | ||
| sensors={sensors} | ||
| onDragStart={handleDragStart} | ||
| onDragEnd={handleDragEnd} | ||
| > | ||
| <SortableContext | ||
| items={containerSortableIds} | ||
| strategy={verticalListSortingStrategy} | ||
| > | ||
| {children} | ||
| </SortableContext> | ||
| <DragOverlay dropAnimation={null}> | ||
| {activeDrag && ( | ||
| <Box | ||
| px="sm" | ||
| py={4} | ||
| style={{ | ||
| background: 'var(--mantine-color-body)', | ||
| border: '1px solid var(--mantine-color-default-border)', | ||
|
alex-fedotyev marked this conversation as resolved.
Outdated
|
||
| borderRadius: 4, | ||
| opacity: 0.85, | ||
| }} | ||
| > | ||
| <Text size="sm" fw={500}> | ||
| {activeDrag.containerTitle} | ||
| </Text> | ||
| </Box> | ||
| )} | ||
| </DragOverlay> | ||
| </DndContext> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -756,6 +756,8 @@ export const TileSchema = z.object({ | |
| h: z.number(), | ||
| config: SavedChartConfigSchema, | ||
| containerId: z.string().optional(), | ||
| // For tiles inside a tab container: which tab this tile belongs to | ||
| tabId: z.string().optional(), | ||
| }); | ||
|
|
||
| export const TileTemplateSchema = TileSchema.extend({ | ||
|
|
@@ -767,11 +769,23 @@ export const TileTemplateSchema = TileSchema.extend({ | |
|
|
||
| export type Tile = z.infer<typeof TileSchema>; | ||
|
|
||
| export const DashboardContainerTabSchema = z.object({ | ||
| id: z.string().min(1), | ||
| title: z.string().min(1), | ||
| }); | ||
|
|
||
| export const DashboardContainerSchema = z.object({ | ||
| id: z.string().min(1), | ||
| type: z.enum(['section']), | ||
| title: z.string().min(1), | ||
| collapsed: z.boolean(), | ||
| // Whether the group can be collapsed (default true) | ||
| collapsible: z.boolean().optional(), | ||
| // Whether to show a border around the group (default true) | ||
| bordered: z.boolean().optional(), | ||
| // Optional tabs: 2+ entries → tab bar renders, 0-1 → plain group header. | ||
| // Tiles reference a specific tab via tabId. | ||
| tabs: z.array(DashboardContainerTabSchema).optional(), | ||
| activeTabId: z.string().optional(), | ||
|
Comment on lines
+781
to
+788
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Has this been tested in a non-preview environment? I worry if there are migrations needed and if the mongoose model needs to be updated.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No migration needed — the Mongoose model stores
Added functional tests that exercise the upgrade path — start with a realistic old-format dashboard (no tabs, no tabId, no collapsible/bordered) and run all hook operations on it: Existing Zod-level test also covers backward compat: |
||
| }); | ||
|
|
||
| export type DashboardContainer = z.infer<typeof DashboardContainerSchema>; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.