refactor(new-webui): Remove redundant StatCard and DetailsCard. - #1088
Conversation
## Walkthrough
This change removes the `StatCard` and `DetailsCard` components, introduces a new `Stat` component, and refactors several IngestPage detail components to use `DashboardCard` with the new `Stat` for displaying statistics. The `Jobs` component interface is simplified, and related component structures are updated accordingly.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|
| `components/webui/client/src/components/Stat/index.tsx` | Added new `Stat` React component for styled statistic display. |
| `components/webui/client/src/components/StatCard/index.tsx` | Deleted `StatCard` component. |
| `components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx` | Deleted `DetailsCard` component. |
| `components/webui/client/src/pages/IngestPage/Details/Files.tsx`<br>`.../Messages.tsx`<br>`.../TimeRange.tsx` | Refactored to replace `DetailsCard` usage with `DashboardCard` and nested new `Stat` component for statistic display. |
| `components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx`<br>`.../UncompressedSize.tsx`<br>`.../index.tsx` | Refactored to use `DashboardCard` and nested `Stat` component instead of `DetailsCard` or `StatCard`. |
| `components/webui/client/src/pages/IngestPage/Jobs/index.tsx` | Simplified: removed `className` prop and interface; passed loading state to `DashboardCard` instead of `VirtualTable`. |
| `components/webui/client/src/pages/IngestPage/index.tsx` | Updated to wrap `Jobs` in a `div` for className styling instead of passing className as a prop. |
| `components/webui/client/src/pages/IngestPage/index.module.css` | Renamed CSS class selector `.jobs` to `.jobsGrid`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant ParentComponent
participant DashboardCard
participant Stat
ParentComponent->>DashboardCard: Render with title, isLoading
DashboardCard->>Stat: Render statistic as child
Stat-->>DashboardCard: Styled statistic display
DashboardCard-->>ParentComponent: Rendered card with statisticPossibly related PRs
Suggested reviewers
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
components/webui/client/src/components/Stat/index.tsx(1 hunks)components/webui/client/src/components/StatCard/index.tsx(0 hunks)components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx(0 hunks)components/webui/client/src/pages/IngestPage/Details/Files.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Details/Messages.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Jobs/index.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(2 hunks)components/webui/client/src/pages/IngestPage/index.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- components/webui/client/src/components/StatCard/index.tsx
- components/webui/client/src/pages/IngestPage/Details/DetailsCard.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/pages/IngestPage/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/Files.tsxcomponents/webui/client/src/pages/IngestPage/Details/TimeRange.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsxcomponents/webui/client/src/pages/IngestPage/Details/Messages.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/webui/client/src/components/Stat/index.tsxcomponents/webui/client/src/pages/IngestPage/Jobs/index.tsx
🧠 Learnings (8)
components/webui/client/src/pages/IngestPage/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/Details/Files.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/components/Stat/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (11)
components/webui/client/src/pages/IngestPage/index.tsx (1)
17-19: Clean refactoring of Jobs component usage.The wrapping of the Jobs component in a div with className is a good approach that aligns with the simplified Jobs interface. This maintains the styling while removing the className prop dependency.
components/webui/client/src/pages/IngestPage/Details/Files.tsx (1)
22-29: Excellent refactoring to dependency injection pattern.The component correctly migrates from DetailsCard to DashboardCard with Stat as a child component. The logic for handling numFiles is preserved, and the new structure aligns with the architectural improvements.
components/webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
22-29: Consistent refactoring implementation.The Messages component follows the same excellent pattern as Files, correctly migrating from DetailsCard to DashboardCard with Stat as a child. The numMessages handling logic is properly preserved.
components/webui/client/src/pages/IngestPage/Jobs/index.tsx (1)
38-48: Well-executed interface simplification.The Jobs component refactoring successfully removes the className prop dependency while maintaining all functionality. Moving the loading state to DashboardCard and applying className directly to VirtualTable is a clean architectural improvement.
components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (1)
21-28: Consistent architectural improvement.The CompressedSize component correctly implements the same dependency injection pattern as other components, migrating from DetailsCard to DashboardCard with Stat as a child. The size formatting logic is properly preserved.
components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (2)
1-2: LGTM: Clean refactoring to dependency injection pattern.The import changes correctly replace the old DetailsCard with DashboardCard and the new Stat component, aligning with the dependency injection refactoring goals.
21-28: LGTM: Proper implementation of new component structure.The refactoring correctly uses DashboardCard as the container and Stat as the child component for displaying the formatted size, maintaining the same functionality while improving the architecture.
components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
3-4: LGTM: Consistent refactoring implementation.The imports correctly align with the new dependency injection pattern, replacing DetailsCard with DashboardCard and introducing the Stat component.
33-40: LGTM: Proper component structure with preserved logic.The component correctly uses DashboardCard as the container and Stat for displaying the time range, while maintaining the existing logic for calculating the stat value.
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (1)
62-74: LGTM: Effective refactoring with proper styling.The component correctly implements the new dependency injection pattern by using DashboardCard as the container and Stat with explicit styling for the percentage display. The backgroundColor and titleColor props maintain the visual design while the nested Stat provides the statistic content.
components/webui/client/src/components/Stat/index.tsx (1)
24-31: LGTM: Well-designed component with proper defaults.The component provides a clean API with sensible defaults using theme tokens. The implementation correctly uses Ant Design's Typography.Text and provides flexibility for customization while maintaining consistency.
StatCard and DetailsCard with dependency injection.StatCard and DetailsCard with dependency injection.
| * A text component for displaying statistical values in `DashboardCard`. | ||
| * | ||
| * @param props | ||
| * @param props.children |
There was a problem hiding this comment.
should this take children or explicit stat string?
There was a problem hiding this comment.
I think junhao wanted me to change this by passing class Name, but I think this is also good with the grid setting explicit on the outside. i am good with this.
There was a problem hiding this comment.
I see... thanks for the context. This makes sense. I made the change because the other grids are explicitly set on the outside. We should check with Junhao later about whether all cards should have a className. He needs this for cloud as well.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/webui/client/src/components/Stat/index.tsx (1)
9-13: Rename interface to match component name.The interface name
StatPropsshould be used instead ofStatCardPropsto match the component nameStatand avoid confusion.-interface StatProps { +interface StatProps {Wait, I see the interface is already named
StatPropsin the current code. This appears to have been addressed from the previous review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
components/webui/client/src/components/Stat/index.tsx(1 hunks)components/webui/client/src/pages/IngestPage/Details/Files.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Details/Messages.tsx(2 hunks)components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx(2 hunks)components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx(2 hunks)components/webui/client/src/pages/IngestPage/index.module.css(1 hunks)components/webui/client/src/pages/IngestPage/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/pages/IngestPage/Details/Files.tsxcomponents/webui/client/src/pages/IngestPage/index.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/index.tsxcomponents/webui/client/src/pages/IngestPage/Details/TimeRange.tsxcomponents/webui/client/src/pages/IngestPage/Details/Messages.tsxcomponents/webui/client/src/components/Stat/index.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsxcomponents/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx
🧠 Learnings (7)
components/webui/client/src/pages/IngestPage/Details/Files.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (6)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
Learnt from: haiqi96
PR: y-scope/clp#523
File: components/core/src/clp/clp/FileCompressor.hpp:58-78
Timestamp: 2024-10-24T14:25:17.978Z
Learning: When reviewing legacy code refactors, avoid suggesting changes that would extend the scope of the PR.
Learnt from: junhaoliao
PR: y-scope/clp#1078
File: components/webui/server/src/plugins/MongoSocketIoServer/index.ts:105-108
Timestamp: 2025-07-08T17:20:02.560Z
Learning: The y-scope/clp codebase has a 100-character line length limit that takes precedence over other style preferences like template literals when they conflict.
components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/components/Stat/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
🔇 Additional comments (12)
components/webui/client/src/pages/IngestPage/index.module.css (1)
10-12: LGTM! Clear and consistent naming change.The class rename from
.jobsto.jobsGridbetter reflects its purpose as a grid container and aligns with the structural changes in the parent component.components/webui/client/src/components/Stat/index.tsx (1)
24-31: LGTM! Well-implemented component with proper styling.The component correctly uses Ant Design's theme tokens for consistent styling and provides sensible defaults for color and fontSize.
components/webui/client/src/pages/IngestPage/Details/Files.tsx (1)
22-28: Excellent refactoring to dependency injection pattern.The change from inheritance-based
DetailsCardto composition usingDashboardCardwithStatchild component successfully implements the dependency injection approach while maintaining the same functionality.components/webui/client/src/pages/IngestPage/Details/Messages.tsx (1)
22-28: Consistent refactoring pattern applied correctly.The component successfully follows the same dependency injection pattern as other detail components, replacing inheritance with composition using
DashboardCardandStat.components/webui/client/src/pages/IngestPage/Details/TimeRange.tsx (2)
3-4: Import statements updated correctly for dependency injection pattern.The imports have been properly updated to use
DashboardCardand the newStatcomponent, aligning with the refactor from inheritance to dependency injection.
33-39: Component structure successfully refactored to use composition.The component now properly uses
DashboardCardas the container withStatas the child component for displaying the formatted time range. This maintains the same functionality while following the new dependency injection pattern.components/webui/client/src/pages/IngestPage/SpaceSavings/CompressedSize.tsx (2)
1-2: Import statements correctly updated for the refactor.The imports follow the same pattern as other refactored components, replacing
DetailsCardwithDashboardCardand adding the newStatcomponent.
21-27: Component structure consistently refactored.The component correctly uses the new composition pattern with
DashboardCardcontaining aStatcomponent that displays the formatted compressed size. TheformatSizeInBytesfunction integration is preserved.components/webui/client/src/pages/IngestPage/SpaceSavings/UncompressedSize.tsx (2)
1-2: Import statements properly updated.The imports are consistent with the refactor pattern, replacing
DetailsCardwithDashboardCardand adding theStatcomponent.
21-27: Component structure successfully refactored.The component correctly implements the new composition pattern, with
DashboardCardcontaining aStatcomponent that displays the formatted uncompressed size. The functionality is preserved.components/webui/client/src/pages/IngestPage/SpaceSavings/index.tsx (2)
4-5: Import statements correctly updated.The imports have been properly updated to use
DashboardCardandStatcomponents, and the import paths are correct with no extra slashes.
62-72: Component structure successfully refactored with proper styling distribution.The component correctly replaces
StatCardwith the composition ofDashboardCardandStat. The styling props are appropriately distributed -backgroundColorandtitleColortoDashboardCard, whilecolorandfontSizeare passed to theStatcomponent. This maintains the same visual appearance while following the dependency injection pattern.
StatCard and DetailsCard with dependency injection.DashboardCard's content via children on IngestPage.
|
@davemarco ready for another review on this. I also tried ai for the PR title -- any better suggestions? |
DashboardCard's content via children on IngestPage.StatCard and DetailsCard.
junhaoliao
left a comment
There was a problem hiding this comment.
(i appreciate this refactoring as it leads us towards making the dashboard customizable)
deferring to @davemarco 's review for approval
Description
As the name suggests, this PR replaces the inheritance style dependency of
Files -> DetailsCard -> StatCard -> DashboardCardwith dependency injection.Checklist
breaking change.
Validation performed
Run
clp-text:hive-24h i-00c90a0f/Run
clp-json:spark-event-logsandpostgresqllogs across several databasesPotential improvements
The height of cards change after the first reloading. I'll create an issue on this and submit another PR.
Summary by CodeRabbit
New Features
Refactor
Chores
.jobsto.jobsGridfor clarity.