-
Notifications
You must be signed in to change notification settings - Fork 109
implement test plans table #4477
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: feature/test-library
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a feature-rich Test Plans page. It adds a data-driven table for displaying test plans, complete with coverage statistics, progress bars, and contextual options. The update includes new React components, SCSS styles, TypeScript types, and localization messages. The page now conditionally renders content based on available test plans and integrates mock data via a custom hook. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestPlansPage
participant useTestPlans
participant TestPlansTable
participant EmptyTestPlans
User->>TestPlansPage: Visit Test Plans page
TestPlansPage->>useTestPlans: Invoke hook to load test plans
useTestPlans-->>TestPlansPage: Return { testPlans, loading }
alt testPlans is empty
TestPlansPage->>EmptyTestPlans: Render empty state
else testPlans available
TestPlansPage->>TestPlansTable: Render table with testPlans data
end
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (6)
app/src/pages/inside/testPlansPage/testPlansTable/progressBar/progressBar.tsx (2)
22-24
: Add input validation and documentation for the progress prop.The
progress
prop should be validated to ensure it's within expected bounds and documented for clarity.interface ProgressBarProps { + /** Progress percentage (0-100) */ - progress: number; + progress: number; }Consider adding runtime validation:
export const ProgressBar = ({ progress }: ProgressBarProps) => ( + const clampedProgress = Math.max(0, Math.min(100, progress)); <div className={cx('progress-bar')}> - <div className={cx('progress-bar__indicator')} style={{ width: `${progress}%` }} /> + <div className={cx('progress-bar__indicator')} style={{ width: `${clampedProgress}%` }} /> </div> );
26-30
: Consider adding accessibility attributes.The progress bar could benefit from ARIA attributes for better accessibility support.
export const ProgressBar = ({ progress }: ProgressBarProps) => ( - <div className={cx('progress-bar')}> + <div className={cx('progress-bar')} role="progressbar" aria-valuenow={progress} aria-valuemin={0} aria-valuemax={100}> <div className={cx('progress-bar__indicator')} style={{ width: `${progress}%` }} /> </div> );app/src/pages/inside/testPlansPage/testPlansTable/messages.ts (1)
19-40
: Optional: derive menu ids from a constant enumTo avoid drift between IDs & keys (see typo above), export an enum of keys and reuse it when defining messages.
app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts (1)
30-30
: Consider reducing the simulation delay for better developer experience.The 2-second delay might be unnecessarily long for development and testing purposes.
- await new Promise((resolve) => setTimeout(resolve, 2000)); + await new Promise((resolve) => setTimeout(resolve, 500));app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.scss (1)
18-22
: Consider adding align-items for complete flexbox centering.If centering is the intended behavior, you might want to add
align-items: center
for vertical centering as well.&__table-container { display: flex; justify-content: center; + align-items: center; padding-top: 26px; }
app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.tsx (1)
100-111
: Consider adding data validation for robustness.The table rendering is implemented correctly. However, consider adding validation to ensure the testPlans data structure is as expected, especially for the coverage field which is used in calculations.
export const TestPlansTable = ({ testPlans }: TestPlansTableProps) => { const { formatMessage } = useIntl(); + // Validate that coverage values are within expected range + const validatedTestPlans = testPlans.filter(plan => + typeof plan.coverage === 'number' && + plan.coverage >= 0 && + plan.coverage <= 100 + ); + - const currentTestPlans = testPlans.map(({ name, coverage, total, covered }) => ({ + const currentTestPlans = validatedTestPlans.map(({ name, coverage, total, covered }) => ({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (19)
app/package.json
(1 hunks)app/src/pages/inside/testPlansPage/commonMessages.ts
(1 hunks)app/src/pages/inside/testPlansPage/emptyTestPlans/emptyTestPlans.tsx
(2 hunks)app/src/pages/inside/testPlansPage/emptyTestPlans/messages.ts
(0 hunks)app/src/pages/inside/testPlansPage/testPlansPage.scss
(2 hunks)app/src/pages/inside/testPlansPage/testPlansPage.tsx
(3 hunks)app/src/pages/inside/testPlansPage/testPlansTable/index.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/messages.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/mockData.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/options/index.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/options/options.scss
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/options/options.tsx
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/progressBar/index.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/progressBar/progressBar.scss
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/progressBar/progressBar.tsx
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.scss
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.tsx
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/types.ts
(1 hunks)app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/pages/inside/testPlansPage/emptyTestPlans/messages.ts
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: Guria
PR: reportportal/service-ui#4403
File: app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateDetails.tsx:30-36
Timestamp: 2025-06-12T00:09:07.409Z
Learning: During the mock stage of UI work in reportportal/service-ui, it is acceptable for interactive elements (e.g., buttons) to omit functional handlers such as `onClick`; these will be implemented in subsequent merge requests. Review comments should avoid flagging these omissions unless the implementation MR is in scope.
Learnt from: allaprischepa
PR: reportportal/service-ui#4418
File: app/localization/translated/zh.json:2125-2130
Timestamp: 2025-06-23T06:22:27.254Z
Learning: For the ReportPortal project, translation work should be delegated to dedicated translators rather than being handled directly during code review. English keys can serve as placeholders until proper translations are provided by the translation team.
app/package.json (2)
Learnt from: AmsterGet
PR: reportportal/service-ui#4397
File: app/src/controllers/plugins/uiExtensions/selectors.js:55-88
Timestamp: 2025-06-10T15:10:26.627Z
Learning: In the ReportPortal service-ui codebase, plugin metadata fields like `details.modules` and `manifest.extensions` are required fields that are validated on the server side, so defensive checks for undefined values are not needed in the client-side selectors.
Learnt from: allaprischepa
PR: reportportal/service-ui#4472
File: app/src/pages/instance/allUsersPage/allUsersHeader/allUsersExport/allUsersExport.tsx:48-50
Timestamp: 2025-07-18T10:24:12.355Z
Learning: In the ReportPortal service-ui project, when removing properties from objects, the `delete` operator is preferred over destructuring when the destructured variable would be unused, as the project uses `@typescript-eslint/no-unused-vars` rule which prevents unused variables from destructuring patterns like `const { limit, ...data } = object;`.
app/src/pages/inside/testPlansPage/emptyTestPlans/emptyTestPlans.tsx (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansPage.scss (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/types.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/progressBar/index.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/options/index.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/commonMessages.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/mockData.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/options/options.tsx (2)
Learnt from: Guria
PR: reportportal/service-ui#4407
File: app/src/pages/common/popoverControl/popoverControl.tsx:55-63
Timestamp: 2025-06-13T21:54:46.839Z
Learning: The Popover component from '@reportportal/ui-kit' handles closing internally when items are clicked, so manual closing via setIsOpened(false) in onClick handlers is not needed.
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:75-86
Timestamp: 2025-06-18T14:36:19.317Z
Learning: In ReportPortal TypeScript components, noop handlers are sometimes used to satisfy TypeScript requirements for required props, not because functionality is missing. This is a common pattern when the actual form handling is managed by wrapper components.
app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.scss (2)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
Learnt from: Guria
PR: reportportal/service-ui#4403
File: app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateDetails.tsx:30-36
Timestamp: 2025-06-12T00:09:07.409Z
Learning: During the mock stage of UI work in reportportal/service-ui, it is acceptable for interactive elements (e.g., buttons) to omit functional handlers such as `onClick`; these will be implemented in subsequent merge requests. Review comments should avoid flagging these omissions unless the implementation MR is in scope.
app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts (1)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
app/src/pages/inside/testPlansPage/testPlansTable/index.ts (3)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
Learnt from: AmsterGet
PR: reportportal/service-ui#4409
File: app/localization/translated/es.json:1648-1652
Timestamp: 2025-06-18T16:24:46.188Z
Learning: In the ReportPortal project, Spanish and Chinese localization entries are intentionally left in English during development. Native speakers handle these translations later in the process, so having English text in Spanish (es.json) and Chinese (zh.json) locale files is expected and acceptable.
Learnt from: allaprischepa
PR: reportportal/service-ui#4472
File: app/src/pages/instance/allUsersPage/allUsersHeader/allUsersExport/allUsersExport.tsx:48-50
Timestamp: 2025-07-18T10:24:12.355Z
Learning: In the ReportPortal service-ui project, when removing properties from objects, the `delete` operator is preferred over destructuring when the destructured variable would be unused, as the project uses `@typescript-eslint/no-unused-vars` rule which prevents unused variables from destructuring patterns like `const { limit, ...data } = object;`.
app/src/pages/inside/testPlansPage/testPlansTable/messages.ts (2)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
Learnt from: AmsterGet
PR: reportportal/service-ui#4409
File: app/localization/translated/es.json:1648-1652
Timestamp: 2025-06-18T16:24:46.188Z
Learning: In the ReportPortal project, Spanish and Chinese localization entries are intentionally left in English during development. Native speakers handle these translations later in the process, so having English text in Spanish (es.json) and Chinese (zh.json) locale files is expected and acceptable.
app/src/pages/inside/testPlansPage/testPlansPage.tsx (6)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:75-86
Timestamp: 2025-06-18T14:36:19.317Z
Learning: In ReportPortal TypeScript components, noop handlers are sometimes used to satisfy TypeScript requirements for required props, not because functionality is missing. This is a common pattern when the actual form handling is managed by wrapper components.
Learnt from: Guria
PR: reportportal/service-ui#4385
File: app/src/components/testCaseList/testCaseNameCell/testCaseNameCell.tsx:30-42
Timestamp: 2025-06-09T17:12:07.281Z
Learning: In React components, static objects like icon mappings that don't depend on props or state should be defined outside the component function to avoid unnecessary re-creation on every render, improving performance.
Learnt from: Guria
PR: reportportal/service-ui#4385
File: app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.scss:17-22
Timestamp: 2025-06-09T17:12:00.851Z
Learning: In the reportportal/service-ui codebase, ScrollWrapper component is used to provide scrollable contexts, eliminating the need to add overflow properties to individual page components for sticky positioning to work correctly.
Learnt from: allaprischepa
PR: reportportal/service-ui#4472
File: app/src/pages/instance/allUsersPage/allUsersHeader/allUsersExport/allUsersExport.tsx:48-50
Timestamp: 2025-07-18T10:24:12.355Z
Learning: In the ReportPortal service-ui project, when removing properties from objects, the `delete` operator is preferred over destructuring when the destructured variable would be unused, as the project uses `@typescript-eslint/no-unused-vars` rule which prevents unused variables from destructuring patterns like `const { limit, ...data } = object;`.
Learnt from: allaprischepa
PR: reportportal/service-ui#4472
File: app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsExport/organizationsExport.tsx:48-50
Timestamp: 2025-07-18T10:24:49.022Z
Learning: In the ReportPortal service-ui project, when the @typescript-eslint/no-unused-vars rule prevents destructuring with unused variables (even with underscore prefixes), the `delete` operator is the preferred approach for removing object properties. This applies specifically when destructuring patterns like `const { limit: _limit, ...data }` still trigger linting errors despite the underscore prefix.
app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.tsx (2)
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:23-24
Timestamp: 2025-06-18T14:37:30.906Z
Learning: In the test case library page at `app/src/pages/inside/testCaseLibraryPage/constans.ts`, the file is intentionally named `constans.ts` (missing the 't'), not `constants.ts`. This is the correct filename and imports referencing '../../../constans' are valid.
Learnt from: yelyzavetakhokhlova
PR: reportportal/service-ui#4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:75-86
Timestamp: 2025-06-18T14:36:19.317Z
Learning: In ReportPortal TypeScript components, noop handlers are sometimes used to satisfy TypeScript requirements for required props, not because functionality is missing. This is a common pattern when the actual form handling is managed by wrapper components.
🧬 Code Graph Analysis (5)
app/src/pages/inside/testPlansPage/emptyTestPlans/emptyTestPlans.tsx (1)
app/src/pages/inside/testPlansPage/commonMessages.ts (1)
commonMessages
(19-32)
app/src/pages/inside/testPlansPage/testPlansTable/options/options.tsx (2)
app/src/pages/common/popoverControl/popoverControl.tsx (1)
PopoverItem
(26-33)app/src/pages/inside/testPlansPage/testPlansTable/messages.ts (1)
messages
(19-40)
app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts (2)
app/src/pages/inside/testPlansPage/testPlansTable/types.ts (1)
TestPlans
(17-22)app/src/pages/inside/testPlansPage/testPlansTable/mockData.ts (1)
mockedTestPlans
(17-36)
app/src/pages/inside/testPlansPage/testPlansTable/messages.ts (1)
app/src/pages/inside/testPlansPage/emptyTestPlans/messages.ts (1)
messages
(19-48)
app/src/pages/inside/testPlansPage/testPlansPage.tsx (4)
app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts (1)
useTestPlans
(21-45)app/src/pages/inside/testPlansPage/commonMessages.ts (1)
commonMessages
(19-32)app/src/pages/inside/testPlansPage/emptyTestPlans/emptyTestPlans.tsx (1)
EmptyTestPlans
(35-69)app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.tsx (1)
TestPlansTable
(35-112)
🔇 Additional comments (20)
app/package.json (1)
30-30
: Dependency @reportportal/ui-kit 0.0.1-alpha.75 availability confirmedVersion 0.0.1-alpha.75 is published on npm alongside the preceding alpha releases (70–74). No further issues detected—proceed with this dependency update.
app/src/pages/inside/testPlansPage/commonMessages.ts (1)
19-32
: Well-structured internationalization implementation.The message definitions follow React Intl best practices with consistent ID naming and clear default messages. Centralizing these messages is a good approach for maintainability.
app/src/pages/inside/testPlansPage/emptyTestPlans/emptyTestPlans.tsx (2)
23-23
: Good refactoring to centralize messages.Adding the import for
commonMessages
supports the message centralization effort and improves consistency across the Test Plans feature.
54-54
: Consistent use of centralized messaging.The change from
messages.createTestPlansLabel
tocommonMessages.createTestPlan
aligns with the centralization effort and ensures consistent labeling across components.app/src/pages/inside/testPlansPage/testPlansPage.scss (2)
19-19
: Appropriate positioning context for action buttons.Adding
position: relative
to the header creates the necessary positioning context for absolutely positioned action buttons.
41-47
: Well-structured action button positioning.The new
.test-plans-page__actions
class provides clean positioning for action buttons with consistent spacing. The absolute positioning within the header and flexbox layout with gap creates a professional layout.app/src/pages/inside/testPlansPage/testPlansTable/options/index.ts (1)
17-17
: LGTM – simple barrel exportNo issues spotted.
app/src/pages/inside/testPlansPage/testPlansTable/progressBar/index.ts (1)
17-17
: LGTM – matches existing barrel patternapp/src/pages/inside/testPlansPage/testPlansTable/mockData.ts (1)
17-36
:coverage
numbers don’t matchcovered/total
Example:
340 / 358 ≈ 95%
, yetcoverage
is98
. Same for the other rows. Either:
- Drop
coverage
and compute it in UI, or- Fix the hard-coded values.
- covered: 340, - total: 358, - coverage: 98, + covered: 340, + total: 358, + coverage: 95,…and so on.
Inconsistent mock data will surface as confusing UI during manual testing.app/src/pages/inside/testPlansPage/testPlansTable/useTestPlans.ts (1)
21-45
: Hook implementation looks good for mock stage development.The
useTestPlans
hook properly simulates async data loading with appropriate TypeScript types and state management. The implementation follows React hooks best practices.app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.scss (1)
17-45
: SCSS implementation follows good practices.The stylesheet uses consistent BEM naming, appropriate CSS custom properties, and provides good interactive states with proper accessibility considerations.
app/src/pages/inside/testPlansPage/testPlansTable/index.ts (1)
17-17
: Clean module export structure.The simple re-export follows standard barrel export patterns and improves module organization by centralizing the component export.
app/src/pages/inside/testPlansPage/testPlansTable/options/options.scss (1)
17-48
: Well-structured button and menu styles.The implementation provides proper button resets, accessible interactive states, and consistent theming through CSS custom properties. The SVG state management is appropriate for the use case.
app/src/pages/inside/testPlansPage/testPlansTable/progressBar/progressBar.scss (1)
17-27
: Clean and functional progress bar implementation.The styles provide a simple, effective progress bar with proper structure, consistent theming, and appropriate visual properties including overflow handling and border radius.
app/src/pages/inside/testPlansPage/testPlansTable/options/options.tsx (1)
30-54
: LGTM! Component structure follows established patterns.The Options component is well-structured and follows the existing codebase patterns. The absence of onClick handlers for menu items is acceptable during the mock stage of UI development, as noted in the retrieved learnings from previous reviews.
app/src/pages/inside/testPlansPage/testPlansPage.tsx (3)
19-19
: Good choice using lodash.isEmpty for consistency.Using
lodash.isEmpty
provides consistent empty checking across different data types compared to manual checks.
28-30
: LGTM! Clean integration of data fetching and components.The integration of the useTestPlans hook and TestPlansTable component is well-structured. Using commonMessages for shared localization keys shows good separation of concerns.
52-63
: Excellent conditional rendering pattern.The conditional rendering logic provides a good user experience:
- Actions are only shown when there are test plans to act upon
- Empty state vs table rendering is handled cleanly
- Button placement and styling follow UI patterns
app/src/pages/inside/testPlansPage/testPlansTable/testPlansTable.tsx (2)
38-58
: Clean data transformation logic.The mapping from test plan data to table rows is well-structured and handles all the required display components appropriately. The use of component cells for complex content (progress bars, options) follows good table design patterns.
60-98
: Well-defined column configuration.The column definitions are comprehensive with appropriate widths, alignment, and headers. The separation between primary and fixed columns follows the Table component's expected API.
app/src/pages/inside/testPlansPage/testPlansTable/options/options.tsx
Outdated
Show resolved
Hide resolved
670228b
to
882afde
Compare
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/test-library #4477 +/- ##
=====================================================
Coverage 66.73% 66.73%
=====================================================
Files 85 85
Lines 995 995
Branches 143 142 -1
=====================================================
Hits 664 664
Misses 300 300
Partials 31 31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR Checklist
develop
for features/bugfixes, other if mentioned in the task)npm run lint
) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build
)?dev
npm script)manage:translations
script?Visuals
Summary by CodeRabbit
New Features
Style
Chores