-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(domains): add grid/table view toggle for domains page #3277
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: canary
Are you sure you want to change the base?
Conversation
Add a view toggle button that allows switching between Grid view (current default) and Table view for better data density and comparison of domain configurations. Table view features: - Sortable Host column with external link - Service, Path, Port, Protocol, Certificate columns - DNS Status column with interactive validation badge - Actions column with DNS helper, Edit, and Delete - Search/filter by hostname - Column visibility dropdown - Pagination controls (Previous/Next) - View preference persisted in localStorage Grid view preserved as default for backward compatibility. Fixes Dokploy#3158
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a table view option to the domains page alongside the existing grid view, with view preference persistence via localStorage. Users can toggle between views using LayoutGrid/LayoutList icons.
Key changes:
- New table view with sortable Host column, filtering, pagination, and column visibility controls
- Added
columns.tsxfile with column definitions using @tanstack/react-table - View toggle button with localStorage persistence for user preference
- DNS validation badge clickable in table view with actions dropdown for Edit/Delete
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
apps/dokploy/components/dashboard/application/domains/columns.tsx |
New file defining 8 table columns (Host, Service, Path, Port, Protocol, Certificate, DNS Status, Actions) with interactive elements and dropdown menu for actions |
apps/dokploy/components/dashboard/application/domains/show-domains.tsx |
Modified to add view toggle button, table implementation with filters/pagination, localStorage hook integration, and refactored delete logic into reusable handler function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| https: domain.https, | ||
| path: domain.path || undefined, | ||
| }} | ||
| serverIp={serverIp} |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DnsHelperModal component renders a Dialog with a DialogTrigger that contains a Button. When placed directly inside DropdownMenuContent, this creates nested interactive elements which can cause accessibility issues and unpredictable behavior. The button inside the DialogTrigger may not function properly within the dropdown menu context, as it conflicts with the dropdown's own focus management and click-away behavior.
Consider wrapping the DnsHelperModal in a DropdownMenuItem or restructuring it to work properly within a dropdown menu context.
| serverIp={serverIp} | |
| serverIp={serverIp} | |
| trigger={ | |
| <Button | |
| variant="ghost" | |
| className="w-full justify-start font-normal h-9 px-2" | |
| > | |
| <Server className="size-4 mr-2" /> | |
| DNS Helper | |
| </Button> | |
| } |
| header: "Port", | ||
| cell: ({ row }) => { | ||
| const port = row.getValue("port") as number | null; | ||
| return <span className="text-sm">{port ?? 3000}</span>; |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The port value falls back to 3000 when null, but this default value might not accurately represent the actual port configuration. In the grid view (line 380), the port is displayed directly without a fallback, showing the actual port value. This inconsistency could mislead users about the actual port being used.
Consider displaying the port as-is (potentially showing null/empty state) or ensure the fallback matches the actual default behavior in the system.
| return <span className="text-sm">{port ?? 3000}</span>; | |
| return <span className="text-sm">{port != null ? port : "-"}</span>; |
| const columns = createColumns({ | ||
| id, | ||
| type, | ||
| validationStates, | ||
| handleValidateDomain, | ||
| handleDeleteDomain, | ||
| isRemoving, | ||
| serverIp, | ||
| }); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The columns are being recreated on every render because createColumns is called directly in the component body. Since validationStates can change frequently (during DNS validation), and handleValidateDomain, handleDeleteDomain, and isRemoving are recreated on each render, this causes the entire column definition array to be recreated unnecessarily, potentially impacting performance.
Consider wrapping createColumns call with useMemo and including only the necessary dependencies, or memoize the handler functions with useCallback.
| column.toggleVisibility(!!value) | ||
| } | ||
| > | ||
| {column.id} |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The column visibility dropdown displays column IDs directly (line 522: {column.id}), which will show technical names like "serviceName", "certificateType", "dnsStatus", and "https" instead of user-friendly labels like "Service Name", "Certificate Type", "DNS Status", and "Protocol". This creates a poor user experience as users need to map technical identifiers to the actual column headers they see in the table.
Consider creating a mapping function or using the column's header property to display proper labels in the visibility dropdown.
| {column.id} | |
| {flexRender(column.columnDef.header, column.getContext())} |
- Fix DnsHelperModal nested button issue by adding `asChild` to DialogTrigger and accepting `children` prop for custom triggers - Fix port display inconsistency: show "-" when null in both grid and table views instead of fallback to 3000 - Add column label mapping for visibility dropdown to show proper labels (Service, Protocol, Certificate, DNS Status) instead of technical IDs (serviceName, https, certificateType, dnsStatus)
Addressed Copilot Review Comments
Why Comment 3 (useMemo optimization) was skipped:
|
Summary
@tanstack/react-tableTable View Columns
Files Changed
apps/dokploy/components/dashboard/application/domains/columns.tsxapps/dokploy/components/dashboard/application/domains/show-domains.tsxTest Plan
Fixes #3158