Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.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 ourterms of service andprivacy 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 andcomparison 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 localStorageGrid view preserved as default for backward compatibility.FixesDokploy#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} |
CopilotAIDec 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" | |
| > | |
| <ServerclassName="size-4 mr-2"/> | |
| DNSHelper | |
| </Button> | |
| } |
| header:"Port", | ||
| cell:({ row})=>{ | ||
| constport=row.getValue("port")asnumber|null; | ||
| return<spanclassName="text-sm">{port??3000}</span>; |
CopilotAIDec 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<spanclassName="text-sm">{port??3000}</span>; | |
| return<spanclassName="text-sm">{port!=null ?port :"-"}</span>; |
| constcolumns=createColumns({ | ||
| id, | ||
| type, | ||
| validationStates, | ||
| handleValidateDomain, | ||
| handleDeleteDomain, | ||
| isRemoving, | ||
| serverIp, | ||
| }); |
CopilotAIDec 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 becausecreateColumns is called directly in the component body. SincevalidationStates can change frequently (during DNS validation), andhandleValidateDomain,handleDeleteDomain, andisRemoving are recreated on each render, this causes the entire column definition array to be recreated unnecessarily, potentially impacting performance.
Consider wrappingcreateColumns call withuseMemo and including only the necessary dependencies, or memoize the handler functions withuseCallback.
| column.toggleVisibility(!!value) | ||
| } | ||
| > | ||
| {column.id} |
CopilotAIDec 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)
Divkix commentedDec 15, 2025
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