Conversation
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
Review Summary by QodoImplement Data Center UI with Environments and Service Specs Management
WalkthroughsDescription• Implemented complete Data Center UI with tabbed layout for Environments and Service Specs management • Created comprehensive CRUD operations (Create, Update, Delete) with modal dialogs for both Environments and Service Specs • Built detail pages for individual environments and service specs with tabbed navigation (Overview, Entities, Request History) • Implemented mock data structures and interfaces for Environment, ServiceSpec, entities, and request history • Added Material-UI styling system with theme-aware dark/light mode support for tables, dialogs, and detail pages • Created reusable form components (EnvironmentFormFields, ServiceSpecFormFields) and shared dialog wrapper (DcmFormDialog) • Implemented search, filtering, and pagination functionality across all table views • Added PatternFly theme synchronization hook to sync Backstage theme with PatternFly dark theme • Created plugin-level router supporting nested routing for detail pages • Added configuration schema for DCM plugin with policy packs support • Updated RHDH logo with dynamic dark theme support • All data is currently mocked as API is not yet ready Diagramflowchart LR
A["DataCenterPage<br/>Tabbed Layout"] --> B["EnvironmentsTabContent<br/>Table + CRUD"]
A --> C["ServiceSpecsTabContent<br/>Table + CRUD"]
B --> D["EnvironmentDetailsPage<br/>Overview/Entities/History"]
C --> E["ServiceSpecDetailsPage<br/>Overview/Entities/History"]
B --> F["RegisterEnvironmentDialog<br/>EnvironmentFormFields"]
B --> G["EditEnvironmentDialog<br/>EnvironmentFormFields"]
B --> H["DeleteEnvironmentDialog"]
C --> I["ServiceSpecCreateDialog<br/>ServiceSpecFormFields"]
C --> J["ServiceSpecEditDialog<br/>ServiceSpecFormFields"]
C --> K["DeleteServiceSpecDialog"]
D --> L["OverviewTab<br/>OverviewField Components"]
D --> M["EntitiesTab<br/>Filtered Table"]
D --> N["RequestHistoryTab<br/>Paginated Table"]
E --> O["SpecOverviewTab<br/>OverviewField Components"]
E --> P["SpecEntitiesTab<br/>Filtered Table"]
E --> Q["SpecRequestHistoryTab<br/>Paginated Table"]
File Changes1. workspaces/dcm/plugins/dcm/src/components/dcmTabTableStyles.ts
|
Code Review by Qodo
1.
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
workspaces/dcm/plugins/dcm/src/pages/data-center/EnvironmentsTabContent.tsx
Show resolved
Hide resolved
asmasarw
force-pushed
the
feature/data-center
branch
3 times, most recently
from
11b6529 to
5b09a9e
Compare
asmasarw
force-pushed
the
feature/data-center
branch
from
cceace7 to
43bffd9
Compare
|
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
-
I can not find form validation logic. Is there a plan to add it? At minimum, disable the submit button when required fields are empty.
-
Explicit setting of fontStyle and similar things can bring headache in the long-run. I would either avoid it or find a theme property or refactor to a more standard backstage component
| @@ -16,14 +16,22 @@ | |||
|
|
|||
| import { makeStyles } from '@material-ui/core'; | |||
|
|
|||
There was a problem hiding this comment.
nitpick: usually nobody except DCM developers care about these packagse/* as nothing will be part of the final plugins' builds.
It's not a mistake. I would just expect it to be exported like in #2631, so it can be reused by the RHDH configuration.
There was a problem hiding this comment.
Are you referring to Export the Icon itself? (RhdhLogoFull) ?
| export const Root = ({ children }: PropsWithChildren<{}>) => { | ||
| const classes = useSidebarItemStyles(); | ||
| const location = useLocation(); | ||
| usePatternFlyTheme(); |
There was a problem hiding this comment.
Make sure you will be able to propagate this logic into RHDH. This function will not be reused there.
| <Route path="/settings" element={<UserSettingsPage />} /> | ||
| <Route path="/catalog-graph" element={<CatalogGraphPage />} /> | ||
| <Route path="/dcm" element={<DcmPage />} /> | ||
| <Route path="/dcm/*" element={<DcmPage />} /> |
There was a problem hiding this comment.
Usually such small changes are ok with this dev-only Backstage instance. Since they have counterparts in the RHDH configuration (which is missing in the Backstage...)
|
|
||
| const useStyles = makeStyles(theme => ({ | ||
| root: { | ||
| paddingLeft: theme.spacing(3), |
There was a problem hiding this comment.
I appreciate using of the theme as much as possible
|
|
||
| import type { DcmEntityRow } from '../data/dcm-mock-rows'; | ||
|
|
||
| export function useDcmEntityListState<T extends DcmEntityRow>(allRows: T[]) { |
There was a problem hiding this comment.
What is the expected extreme scale (count of records) for this table?
If it is high, mind filtering/paginating on the backend side.
| ); | ||
| const classes = useDcmDetailsPageStyles(); | ||
|
|
||
| const spec = useMemo( |
There was a problem hiding this comment.
This will mecome stale after first read. I guess this is not expected once there are non-mocked data. What about useEffect/useState?
| const handleDeleteConfirm = useCallback(() => { | ||
| if (envToDelete) { | ||
| setEnvironments(prev => prev.filter(e => e.id !== envToDelete.id)); | ||
| setPage(prev => Math.max(0, prev - 1)); |
There was a problem hiding this comment.
Is that as expected? To remove the page after deleting an environment?
| @@ -0,0 +1,359 @@ | |||
| /* | |||
There was a problem hiding this comment.
This EnvironmentsTabContent file shares a lot of content with the ServiceSpecsTabContent.
Can it be reused?
| @@ -0,0 +1,73 @@ | |||
| /* | |||
There was a problem hiding this comment.
Very similar to SpecEntitiesTab, can it be reused?
| const primaryMain = | ||
| (p.primary && (p.primary as { main?: string }).main) || '#1976d2'; | ||
| return { | ||
| '--dcm-tab-color-unselected': isDark |
There was a problem hiding this comment.
This looks scary. What about using makeStyles instead?
| field: 'component', | ||
| cellStyle: { minWidth: 140 }, | ||
| render: data => ( | ||
| <Link to="#" style={{ textDecoration: 'underline' }}> |
There was a problem hiding this comment.
what is the purpose of this link-to-nowhere?
| @@ -5,19 +5,35 @@ | |||
| ```ts | |||
There was a problem hiding this comment.
Make sure this is generated by yarn build:api-reports only. Use backstage-cli for that
There was a problem hiding this comment.
This is generated by 'yarn build:api-reports'.
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
-
There is no changeset
-
missing tests, do you plan to add them?
|
Hi @mareklibra a few points to mention:
|
2 participants
Implement Data-Center UI including:
** All Data are mocked since API is NOT ready yet.
UI Images: