-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore(ui): Revamp Test case details page #25118
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?
Conversation
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 revamps the test case details page UI to improve visual consistency and user experience. The changes unify the parameter display by merging parameters and compute row count into a single section, improve graph readability with rotated X-axis labels, redesign the tooltip with a cleaner structure, and apply consistent styling across all components.
Key changes:
- Unified Parameter Display: Merged parameters and compute row count into a single "Parameter" section with a 2-column grid layout
- Improved Graph Readability: Rotated X-axis labels -45° to prevent overlap on the test result timeline
- Enhanced Tooltip: Restructured with date-time header, color-coded status indicators, and row metrics displayed as fractions
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| EntityVersionUtils.tsx | Updated CSS class names for parameter labels and values to use new styling classes |
| description-v1.less | Added 10px border-radius to description card for visual consistency |
| test-summary-custom-tooltip.less | Added tooltip styling for date-time header, separators, and increased max-height |
| TestSummaryCustomTooltip.test.tsx | Updated test expectations to match new tooltip structure (rows displayed as fractions) |
| TestSummaryCustomTooltip.component.tsx | Refactored tooltip to show date-time header, color-coded status, and rows as fractions (e.g., "4/6") |
| TestSummaryGraph.tsx | Added -45° angle rotation to X-axis labels with increased bottom margin for readability |
| incident-manager.style.less | Removed background and padding from parameter-value-container for cleaner design |
| test-case-result-tab.style.less | Added comprehensive parameter section styles with grid layout, labels, and SQL expression container |
| TestCaseResultTab.test.tsx | Updated tests to match unified parameter container structure |
| TestCaseResultTab.component.tsx | Major refactoring: merged parameter and compute row count sections, added 2-column grid layout with chunk-based row rendering |
...src/components/DataQuality/IncidentManager/TestCaseResultTab/TestCaseResultTab.component.tsx
Outdated
Show resolved
Hide resolved
...src/components/DataQuality/IncidentManager/TestCaseResultTab/TestCaseResultTab.component.tsx
Show resolved
Hide resolved
...src/components/DataQuality/IncidentManager/TestCaseResultTab/TestCaseResultTab.component.tsx
Show resolved
Hide resolved
...components/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsx
Show resolved
Hide resolved
...components/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsx
Show resolved
Hide resolved
🔍 CI failure analysis for 031a53e: Multiple Playwright CI jobs (3 different matrix configurations) are failing with the same infrastructure and flakiness patterns, confirming systematic test environment issues unrelated to PR changes.IssueThree different Playwright CI matrix configurations have now failed:
All three show the same infrastructure and flakiness patterns. Root CauseConsistent Infrastructure Issues Across All Jobs:
Consistent Test Failure Patterns:
AnalysisThis is a systematic test environment problem affecting multiple matrix configurations:
None of the failing tests exercise these modified components. DetailsWhy Multiple Matrix Configurations Are Failing:
Pattern Confirmation:
Code Review 👍 Approved with suggestionsClean UI revamp with well-structured component refactoring. Previous minor edge case findings remain unaddressed but are low-risk. Suggestions 💡 2 suggestionsEdge Case: Tooltip status color undefined for non-Failed/Success statusesIn const statusColor =
status === TestCaseStatus.Failed
? RED_3
: status === TestCaseStatus.Success
? GREEN_3
: undefined;Suggested fix: Consider adding explicit colors for other status types (e.g., yellow/orange for Warning, grey for Aborted/Queued) to ensure consistent visual feedback across all possible statuses. Edge Case: totalRows calculation may hide row data when passedRows/failedRows are 0The For example, if More importantly, if one of const hasRowData = passedRows !== undefined || failedRows !== undefined;And use this alongside the Resolved ✅ 3 resolvedEdge Case: Missing key prop on divider inside rows.map
Bug: renderParameterRows function causes stale closure in useMemo
Code Quality: Removed data-testid for computed-row-count-container
What Works WellGood use of lodash's Tip Comment OptionsAuto-apply is off Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs) |
|
ShaileshParmar11
left a comment
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.
since you are working on test case details page revamp, lets migrate it to MUI base structure
...components/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsx
Show resolved
Hide resolved
...components/Database/Profiler/TestSummaryCustomTooltip/TestSummaryCustomTooltip.component.tsx
Show resolved
Hide resolved
| import chunk from 'lodash/chunk'; | ||
| import isEmpty from 'lodash/isEmpty'; | ||
| import isUndefined from 'lodash/isUndefined'; | ||
| import startCase from 'lodash/startCase'; | ||
| import toString from 'lodash/toString'; |
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.
any reason for this change?
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.
Path imports (lodash/startCase) are better because they guarantee minimal bundle size and avoid tree-shaking uncertainty, while named imports from lodash may accidentally include the entire library.
...src/components/DataQuality/IncidentManager/TestCaseResultTab/TestCaseResultTab.component.tsx
Show resolved
Hide resolved
| <div key={rowKey}> | ||
| <Row className="parameter-row" gutter={[16, 0]}> | ||
| {row.map((item) => ( | ||
| <Col key={item.label} span={row.length === 1 ? 16 : 12}> | ||
| <Space align="start" size={4}> | ||
| <Typography.Text className="parameter-label"> | ||
| {`${item.label}:`} | ||
| </Typography.Text> |
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.
We should use MUI instead of antd



Describe your changes:
Fixes #23396
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
TestCaseResultTab.component.tsxTestSummaryGraph.tsxto prevent label overlap on test result timelineTestSummaryCustomTooltip.component.tsxwith date-time header, color-coded status (red/green), and row metrics displayed as fractionsThis will update automatically on new commits.