Feat/heroui v3 upgrade - #5196
Draft
Adarshkumar0509 wants to merge 15 commits into
Draft
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughUpgrades HeroUI to v3, migrates frontend components to updated APIs, revises navigation and pagination, updates styling and providers, and adjusts Jest mocks, selectors, and ESLint configuration. ChangesHeroUI v3 migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
2 similar comments
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5196 +/- ##
==========================================
- Coverage 98.83% 98.83% -0.01%
==========================================
Files 538 538
Lines 17123 17104 -19
Branches 2460 2458 -2
==========================================
- Hits 16924 16905 -19
Misses 99 99
Partials 100 100
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
306-357: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the pagination logic from the JSX IIFE.
The immediately-invoked function expression (IIFE) inside the JSX makes the render block harder to read. Consider moving
currentPage,totalPages, andgoToPageto the main component body, placing them just before thereturnstatement.♻️ Proposed refactor
Extract the logic to the component body:
const currentPage = getCurrentPage() const totalPages = Math.ceil(metricsLength / PAGINATION_LIMIT) const goToPage = async (page: number) => { const newOffset = (page - 1) * PAGINATION_LIMIT const newPagination = { offset: newOffset, limit: PAGINATION_LIMIT } setPagination(newPagination) await fetchMore({ variables: { filters, pagination: newPagination, ordering: buildOrderingWithTieBreaker(ordering), }, updateQuery: (prev, { fetchMoreResult }) => { if (!fetchMoreResult) return prev return { ...prev, projectHealthMetrics: fetchMoreResult.projectHealthMetrics, } }, }) } return ( // ... rest of the componentAnd then in your JSX:
- {(() => { - const currentPage = getCurrentPage() - const totalPages = Math.ceil(metricsLength / PAGINATION_LIMIT) - const goToPage = async (page: number) => { - // ... - } - return ( <Pagination className="mt-4"> <Pagination.Content> <Pagination.Item> <Pagination.Previous isDisabled={currentPage === 1} onPress={() => goToPage(currentPage - 1)} > <Pagination.PreviousIcon /> </Pagination.Previous> </Pagination.Item> {Array.from({ length: totalPages }, (_, i) => i + 1).map((p) => ( <Pagination.Item key={p}> <Pagination.Link isActive={currentPage === p} onPress={() => goToPage(p)}> {p} </Pagination.Link> </Pagination.Item> ))} <Pagination.Item> <Pagination.Next isDisabled={currentPage === totalPages} onPress={() => goToPage(currentPage + 1)} > <Pagination.NextIcon /> </Pagination.Next> </Pagination.Item> </Pagination.Content> </Pagination> - ) - })()}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/projects/dashboard/metrics/page.tsx` around lines 306 - 357, Extract currentPage, totalPages, and goToPage from the JSX IIFE into the main component body immediately before the return statement. Keep goToPage’s pagination state updates, fetchMore variables, and updateQuery behavior unchanged, then render the existing Pagination markup directly without the IIFE wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eslint.config.mjs`:
- Around line 222-227: Re-enable the import/order rule in the frontend override
by removing its off setting from the rules object, then run ESLint with --fix
across the frontend TypeScript and JavaScript files to resolve all resulting
import-order violations.
In `@frontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsx`:
- Around line 5-17: Restore interaction coverage in the FormDateInput test by
forwarding TextField’s value and onChange through its child render context to
Input, and have Input use the received handler instead of a no-op. Update the
test named “calls onValueChange when input changes” to fire a change event and
assert onValueChange is called, following the existing CreateModule.test.tsx
mock pattern.
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx`:
- Around line 262-298: Fix the ESLint directive placement around the
`@heroui/select` mock: remove the unmatched eslint-enable or add a matching
`@typescript-eslint/no-explicit-any` disable immediately before the mock and
retain the enable after it. Ensure all any usages in the Select and SelectItem
mock remain covered without leaving unmatched directives.
- Around line 285-292: Update the Select mock’s children mapping to derive each
option’s value from the child SelectItem’s own key or id instead of the
positional levels array. Preserve the child label rendering while ensuring
reordered EXPERIENCE_LEVELS in ModuleForm does not mismatch option values and
labels.
In `@frontend/pnpm-workspace.yaml`:
- Around line 44-45: Update the `@heroui/styles` entry in the pnpm workspace
exclusion list from 3.2.2 to 3.2.1 so it matches the version specified in
package.json and keeps the release age check aligned.
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 339-345: Replace the full-page mapping in the pagination component
with truncated page-item generation, using HeroUI’s usePagination hook or
equivalent custom logic. Preserve active-page selection and goToPage handling
for numeric items, while rendering ellipsis states appropriately so large page
counts do not produce a link for every page.
In `@frontend/src/components/ModuleForm.tsx`:
- Line 375: Remove the unused _isLoading state and setIsLoading setter from
ModuleForm, then remove all corresponding updates in the project-fetch logic and
related paths. Preserve the existing fetch and search behavior without
introducing replacement loading state.
---
Outside diff comments:
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 306-357: Extract currentPage, totalPages, and goToPage from the
JSX IIFE into the main component body immediately before the return statement.
Keep goToPage’s pagination state updates, fetchMore variables, and updateQuery
behavior unchanged, then render the existing Pagination markup directly without
the IIFE wrapper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26ca3047-044c-463e-86d7-127457da9e0f
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
eslint.config.mjsfrontend/__tests__/a11y/components/ChapterMap.a11y.test.tsxfrontend/__tests__/unit/components/BreadCrumbs.test.tsxfrontend/__tests__/unit/components/ChapterMap.test.tsxfrontend/__tests__/unit/components/EntityActions.test.tsxfrontend/__tests__/unit/components/HealthMetrics.test.tsxfrontend/__tests__/unit/components/IssuesTable.test.tsxfrontend/__tests__/unit/components/ModuleCard.test.tsxfrontend/__tests__/unit/components/ModuleForm.test.tsxfrontend/__tests__/unit/components/PageLayout.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsxfrontend/__tests__/unit/components/UserCard.test.tsxfrontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/Header.test.tsxfrontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsxfrontend/jest.config.tsfrontend/package.jsonfrontend/pnpm-workspace.yamlfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/BreadCrumbs.tsxfrontend/src/components/BreadCrumbsWrapper.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/ProjectsDashboardDropDown.tsxfrontend/src/components/ProjectsDashboardNavBar.tsxfrontend/src/components/forms/shared/FormDateInput.tsxfrontend/src/components/forms/shared/FormTextInput.tsxfrontend/src/wrappers/provider.tsx
💤 Files with no reviewable changes (8)
- frontend/tests/unit/components/UserCard.test.tsx
- frontend/tests/unit/pages/ApiKeysPage.test.tsx
- frontend/tests/unit/components/ModuleCard.test.tsx
- frontend/tests/a11y/components/ChapterMap.a11y.test.tsx
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/tests/unit/components/ChapterMap.test.tsx
- frontend/tests/unit/components/EntityActions.test.tsx
- frontend/tests/unit/pages/Header.test.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 31 files
Confidence score: 3/5
- In
frontend/src/app/projects/dashboard/metrics/page.tsx, rendering aPagination.Linkfor every page can create an extremely long pagination control on large datasets, which is likely to hurt usability and potentially UI performance after merge; switch to truncated pagination (with ellipsis/neighbor limits) before merging to keep navigation scalable.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/projects/dashboard/metrics/page.tsx">
<violation number="1" location="frontend/src/app/projects/dashboard/metrics/page.tsx:339">
P1: This renders a `Pagination.Link` for every single page without truncation. For datasets with many pages, this will display a massive list of page links instead of using ellipsis (e.g., `1 2 3 ... 10`). HeroUI v3's compound Pagination API provides a `Pagination.Ellipsis` sub-component specifically for this, but truncation logic must be implemented manually. Consider adding custom range/truncation logic that inserts `<Pagination.Ellipsis />` when the page count exceeds a threshold.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Collaborator
Author
|
wip |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
309-326: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep pagination state in sync with
fetchMore.setPagination(newPagination)runs beforeawait fetchMore(...), so a network/GraphQL error can leave the UI on a page whose metrics never loaded and bubble an unhandled rejection. Move the state update afterfetchMoresucceeds, or wrap the call intry/catchand revert/report on failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/projects/dashboard/metrics/page.tsx` around lines 309 - 326, Update the goToPage pagination flow so setPagination(newPagination) occurs only after fetchMore completes successfully, or handle fetchMore failures with try/catch and restore/report the error. Keep the existing fetchMore variables and updateQuery behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 309-326: Update the goToPage pagination flow so
setPagination(newPagination) occurs only after fetchMore completes successfully,
or handle fetchMore failures with try/catch and restore/report the error. Keep
the existing fetchMore variables and updateQuery behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b3215dc8-b6f7-4d19-9d61-912046b3464d
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
frontend/package.jsonfrontend/src/app/globals.cssfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/ProjectsDashboardNavBar.tsxfrontend/tailwind.config.mjs
💤 Files with no reviewable changes (1)
- frontend/src/components/ModuleForm.tsx
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Adarshkumar0509
force-pushed
the
feat/heroui-v3-upgrade
branch
from
July 14, 2026 09:21
dd62648 to
7d02cb3
Compare
|
Contribution validation failed:
|
1 similar comment
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
eslint.config.mjs (1)
222-227: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRe-enable
import/orderand fix violations automatically.Disabling
import/orderfor the entirefrontenddirectory degrades code maintainability and allows inconsistent imports to accumulate. Instead of turning off the rule, consider keeping it enabled and runningeslint --fixto automatically resolve any import order issues introduced during the migration.♻️ Proposed fix
- { - files: ['frontend/**/*.{ts,tsx,js,jsx}'], - rules: { - 'import/order': 'off', - }, - },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eslint.config.mjs` around lines 222 - 227, Remove the frontend-specific `'import/order': 'off'` override from the ESLint configuration so the rule remains enabled for `frontend/**/*.{ts,tsx,js,jsx}`. Run ESLint with `--fix` across the affected frontend files and retain the resulting import-order corrections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ModuleForm.tsx`:
- Around line 329-333: Complete the HeroUI 3.2 Switch composition migration: in
frontend/src/components/ModuleForm.tsx lines 329-333, wrap Switch.Control and
Switch.Thumb with Switch.Content; in
frontend/__tests__/unit/components/ModuleForm.test.tsx lines 142-168 and
frontend/__tests__/unit/pages/CreateModule.test.tsx lines 232-258, add Content
to the Switch mocks so they match the updated API.
- Around line 451-480: The ComboBox in ModuleForm must remain open while
debounced suggestions are loading. Add the allowsEmptyCollection prop to the
ComboBox containing projectSelector, preserving the existing behavior and other
props.
---
Duplicate comments:
In `@eslint.config.mjs`:
- Around line 222-227: Remove the frontend-specific `'import/order': 'off'`
override from the ESLint configuration so the rule remains enabled for
`frontend/**/*.{ts,tsx,js,jsx}`. Run ESLint with `--fix` across the affected
frontend files and retain the resulting import-order corrections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a47aa6c0-6c13-4403-91d3-c1d3f5e5d2fb
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
eslint.config.mjsfrontend/__tests__/a11y/components/ChapterMap.a11y.test.tsxfrontend/__tests__/unit/components/ChapterMap.test.tsxfrontend/__tests__/unit/components/EntityActions.test.tsxfrontend/__tests__/unit/components/HealthMetrics.test.tsxfrontend/__tests__/unit/components/IssuesTable.test.tsxfrontend/__tests__/unit/components/ModuleCard.test.tsxfrontend/__tests__/unit/components/ModuleForm.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsxfrontend/__tests__/unit/components/UserCard.test.tsxfrontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/Header.test.tsxfrontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsxfrontend/jest.config.tsfrontend/package.jsonfrontend/pnpm-workspace.yamlfrontend/src/app/globals.cssfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/ProjectsDashboardDropDown.tsxfrontend/src/components/ProjectsDashboardNavBar.tsxfrontend/src/components/forms/shared/FormDateInput.tsxfrontend/src/components/forms/shared/FormTextInput.tsxfrontend/src/wrappers/provider.tsxfrontend/tailwind.config.mjs
💤 Files with no reviewable changes (8)
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/tests/unit/components/ChapterMap.test.tsx
- frontend/tests/unit/pages/ApiKeysPage.test.tsx
- frontend/tests/unit/components/UserCard.test.tsx
- frontend/tests/a11y/components/ChapterMap.a11y.test.tsx
- frontend/tests/unit/components/EntityActions.test.tsx
- frontend/tests/unit/components/ModuleCard.test.tsx
- frontend/tests/unit/pages/Header.test.tsx
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 7 unresolved issues from previous reviews.
Re-trigger cubic
Adarshkumar0509
force-pushed
the
feat/heroui-v3-upgrade
branch
from
July 14, 2026 21:42
37f4637 to
1731e20
Compare
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ProjectsDashboardDropDown.tsx (1)
44-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
<button type="button">over<div>for dropdown triggers.Using a
<div>for a trigger relies entirely on the UI library to inject accessibility attributes (liketabIndex,role="button", and keyboard event handlers). Using a semantic<button type="button">natively ensures full keyboard accessibility, focus management, and correct screen reader interpretation without any polyfilling.♻️ Proposed refactor
- <div className="flex cursor-pointer items-center gap-2"> + <button type="button" className="flex cursor-pointer items-center gap-2 bg-transparent border-none p-0 text-left"> <IconWrapper icon={ isOrdering ? orderingIconsMapping[selectedKeys?.[0] as keyof typeof orderingIconsMapping] || FaArrowDownWideShort : icon || FaArrowDownWideShort } /> <div className="flex flex-col items-center"> <span className="text-md">{buttonDisplayName}</span> {selectedLabels && selectedLabels.length > 0 && ( <span className="text-xs text-gray-500 dark:text-gray-400"> {selectedLabels.join(', ')} </span> )} </div> - </div> + </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProjectsDashboardDropDown.tsx` around lines 44 - 61, Replace the dropdown trigger wrapper div in ProjectsDashboardDropDown with a semantic button using type="button". Preserve the existing classes and child content, and ensure the dropdown interaction remains attached to this button so native keyboard focus and activation work correctly.
♻️ Duplicate comments (1)
eslint.config.mjs (1)
222-227: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRe-enable
import/orderand fix violations automatically.Disabling
import/orderfor the entirefrontenddirectory degrades code maintainability and allows inconsistent imports to accumulate. Instead of turning off the rule, consider keeping it enabled and runningeslint --fixto automatically resolve any import order issues introduced during the migration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eslint.config.mjs` around lines 222 - 227, Remove the frontend-specific 'import/order': 'off' override from the ESLint configuration so the rule remains enabled for frontend files, then run ESLint with --fix to resolve all resulting import-order violations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eslint.config.mjs`:
- Around line 228-237: Remove the broad test-file rule overrides in the ESLint
configuration, especially for `@typescript-eslint/no-explicit-any`,
`@typescript-eslint/no-unused-vars`, import/no-duplicates, and related TypeScript
rules. Fix resulting violations in the affected tests, using narrowly scoped
inline disables only where a violation is intentional and unavoidable.
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx`:
- Around line 374-377: Add the Experience Level selection setup to the
submission-path tests around the test cases at lines 576-591 and 628-642, using
the existing combobox lookup and selecting “BEGINNER” before submission. Ensure
both tests populate this required field so their mocked mutations are invoked.
---
Outside diff comments:
In `@frontend/src/components/ProjectsDashboardDropDown.tsx`:
- Around line 44-61: Replace the dropdown trigger wrapper div in
ProjectsDashboardDropDown with a semantic button using type="button". Preserve
the existing classes and child content, and ensure the dropdown interaction
remains attached to this button so native keyboard focus and activation work
correctly.
---
Duplicate comments:
In `@eslint.config.mjs`:
- Around line 222-227: Remove the frontend-specific 'import/order': 'off'
override from the ESLint configuration so the rule remains enabled for frontend
files, then run ESLint with --fix to resolve all resulting import-order
violations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f51d6d6-7096-451d-9cd6-cab9c082ced1
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
eslint.config.mjsfrontend/__tests__/a11y/components/ChapterMap.a11y.test.tsxfrontend/__tests__/unit/components/ChapterMap.test.tsxfrontend/__tests__/unit/components/EntityActions.test.tsxfrontend/__tests__/unit/components/HealthMetrics.test.tsxfrontend/__tests__/unit/components/IssuesTable.test.tsxfrontend/__tests__/unit/components/ModuleCard.test.tsxfrontend/__tests__/unit/components/ModuleForm.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsxfrontend/__tests__/unit/components/UserCard.test.tsxfrontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/Header.test.tsxfrontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsxfrontend/jest.config.tsfrontend/package.jsonfrontend/src/app/globals.cssfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/ProjectsDashboardDropDown.tsxfrontend/src/components/ProjectsDashboardNavBar.tsxfrontend/src/components/forms/shared/FormDateInput.tsxfrontend/src/components/forms/shared/FormTextInput.tsxfrontend/src/wrappers/provider.tsxfrontend/tailwind.config.mjs
💤 Files with no reviewable changes (8)
- frontend/tests/unit/components/UserCard.test.tsx
- frontend/tests/unit/pages/ApiKeysPage.test.tsx
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/tests/unit/components/EntityActions.test.tsx
- frontend/tests/unit/components/ChapterMap.test.tsx
- frontend/tests/unit/pages/Header.test.tsx
- frontend/tests/unit/components/ModuleCard.test.tsx
- frontend/tests/a11y/components/ChapterMap.a11y.test.tsx
Adarshkumar0509
marked this pull request as ready for review
July 14, 2026 23:06
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
frontend/src/components/forms/shared/FormButtons.tsx,SubmitButtonhard-codes Tailwind classes that duplicate HeroUI v3 button styling, which can cause style drift or conflicting states (hover/disabled/spacing) as the design system evolves; rely on HeroUI variants/tokens instead and remove the duplicated utility classes to de-risk regressions.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/forms/shared/FormButtons.tsx">
<violation number="1" location="frontend/src/components/forms/shared/FormButtons.tsx:36">
P2: The Submit Button's className includes explicit Tailwind classes (`bg-blue-600`, `hover:bg-blue-700`, `rounded-md`, `px-4`, `py-2`, `font-medium`, `text-white`, `disabled:opacity-50`) that duplicate HeroUI v3's built-in styling for `variant="solid"` with `color="primary"`. This defeats HeroUI's theming — if the design system's primary color changes, this Button won't inherit the update — and the duplicate classes add unnecessary maintenance surface. Remove the className or keep only truly custom overrides that HeroUI's slot system doesn't provide.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
…teractive Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
Adarshkumar0509
force-pushed
the
feat/heroui-v3-upgrade
branch
from
July 25, 2026 12:17
569eae2 to
80c2ac9
Compare
|
Contribution validation failed:
|
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
|
Contribution validation failed:
|
Adarshkumar0509
marked this pull request as draft
July 26, 2026 02:45
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
frontend/__tests__/unit/pages/CreateModule.test.tsx (1)
263-296: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStray
// eslint-enablewith no matching disable, while theanyusages below are unguarded.
//-styleeslint-enableisn't a directive at all, and there is no precedingeslint-disablein this file — so theanyannotations at Lines 275, 286 and 295 remain exposed to@typescript-eslint/no-explicit-any. AlsoSelectItemdestructureschildrenbut returnsnull.♻️ Proposed fix
-// eslint-enable `@typescript-eslint/no-explicit-any` - +/* eslint-disable `@typescript-eslint/no-explicit-any` */ jest.mock('`@heroui/select`', () => ({ @@ - SelectItem: ({ children }: any) => null, + SelectItem: () => null, })) +/* eslint-enable `@typescript-eslint/no-explicit-any` */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/__tests__/unit/pages/CreateModule.test.tsx` around lines 263 - 296, Fix the mock declarations in the `@heroui/select` Jest mock by removing the stray `// eslint-enable` and adding a correctly scoped ESLint disable directive for the intentional `any` annotations, then re-enable the rule afterward. Update `SelectItem` so its `children` prop is used to render the option content instead of being destructured and discarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsx`:
- Around line 5-38: Update the mocked TextField in FormDateInput tests to accept
its id prop and render that id on the mock wrapper, matching the real TextField
behavior. Keep the existing value, validation, and child-propagation behavior
unchanged so duplicate IDs can be detected once the component is corrected.
In `@frontend/__tests__/unit/components/ModuleForm.test.tsx`:
- Around line 29-51: The ComboBox mock in ModuleForm.test.tsx still uses stale
autocomplete test IDs; rename autocomplete and autocomplete-input to the
corresponding combobox-* IDs, matching the existing combobox-children naming and
updating any references in the test.
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx`:
- Around line 13-16: Update the `@heroui/react` mock in CreateModule.test.tsx to
avoid spreading jest.requireActual('`@heroui/react`'). Mock only the HeroUI
components and exports used by this test, while preserving the existing
React-based behavior and required symbols.
- Around line 174-203: Update the mocked Item component in CreateModule tests so
the {...props} spread occurs before the explicit onClick and onKeyDown handlers,
ensuring forwarded props cannot override selection behavior while preserving the
existing handler logic.
In `@frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx`:
- Around line 52-56: Update the Next mock in ProjectsHealthDashboardMetrics
tests to render its children instead of hardcoding “Next Page,” and adjust the
related assertions to query the control by role or aria-label so the test
validates the component’s actual label.
In `@frontend/package.json`:
- Line 99: Update the graphql-codegen script to reference the tracked
configuration filename graphql-codegen-config.ts, or rename that tracked file to
graphql-codegen.ts while keeping the script consistent. Ensure pnpm
graphql-codegen resolves the active configuration.
In `@frontend/src/components/forms/shared/FormTextInput.tsx`:
- Around line 33-59: The FormTextInput TextField layout lost its shared spacing
and input styling. Update the TextField className to restore the vertical stack
with appropriate gap, and update the Input className to include the previous
border and rounding classes while preserving the existing width, background, and
text styles; apply the same restoration in FormDateInput if it uses the
corresponding shared styling.
- Around line 35-53: Remove the duplicate id prop from the TextField component
while retaining id={id} on the nested Input in both
frontend/src/components/forms/shared/FormTextInput.tsx (lines 35-53) and
frontend/src/components/forms/shared/FormDateInput.tsx (lines 29-46), so Label
htmlFor continues targeting the input element.
In `@frontend/src/components/ModuleForm.tsx`:
- Around line 319-325: Update the Switch usage in ModuleForm to use the HeroUI
v3 composition: nest Switch.Content, Switch.Control, and Switch.Thumb inside the
existing Switch while preserving its aria-label, isSelected binding, and
onChange handler.
In `@frontend/src/components/ProjectsDashboardDropDown.tsx`:
- Around line 47-51: Update the onKeyDown handler in ProjectsDashboardDropDown
to call preventDefault() when handling Enter or Space before triggering the
click, preventing Space from scrolling while preserving dropdown activation.
---
Duplicate comments:
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx`:
- Around line 263-296: Fix the mock declarations in the `@heroui/select` Jest
mock by removing the stray `// eslint-enable` and adding a correctly scoped
ESLint disable directive for the intentional `any` annotations, then re-enable
the rule afterward. Update `SelectItem` so its `children` prop is used to render
the option content instead of being destructured and discarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 34d2aee9-c622-4419-9cae-b4efffafdf67
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
eslint.config.mjsfrontend/__tests__/a11y/components/ChapterMap.a11y.test.tsxfrontend/__tests__/unit/components/BreadCrumbs.test.tsxfrontend/__tests__/unit/components/ChapterMap.test.tsxfrontend/__tests__/unit/components/EntityActions.test.tsxfrontend/__tests__/unit/components/HealthMetrics.test.tsxfrontend/__tests__/unit/components/IssuesTable.test.tsxfrontend/__tests__/unit/components/ModuleCard.test.tsxfrontend/__tests__/unit/components/ModuleForm.test.tsxfrontend/__tests__/unit/components/PageLayout.test.tsxfrontend/__tests__/unit/components/ProgramCard.test.tsxfrontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsxfrontend/__tests__/unit/components/UserCard.test.tsxfrontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/CreateModule.test.tsxfrontend/__tests__/unit/pages/Header.test.tsxfrontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsxfrontend/jest.config.tsfrontend/package.jsonfrontend/pnpm-workspace.yamlfrontend/src/app/globals.cssfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/BreadCrumbs.tsxfrontend/src/components/BreadCrumbsWrapper.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/ProjectsDashboardDropDown.tsxfrontend/src/components/ProjectsDashboardNavBar.tsxfrontend/src/components/forms/shared/FormButtons.tsxfrontend/src/components/forms/shared/FormDateInput.tsxfrontend/src/components/forms/shared/FormTextInput.tsxfrontend/src/wrappers/provider.tsxfrontend/tailwind.config.mjs
💤 Files with no reviewable changes (8)
- frontend/tests/unit/components/EntityActions.test.tsx
- frontend/tests/unit/components/UserCard.test.tsx
- frontend/tests/unit/components/ProgramCard.test.tsx
- frontend/tests/unit/pages/ApiKeysPage.test.tsx
- frontend/tests/a11y/components/ChapterMap.a11y.test.tsx
- frontend/tests/unit/components/ModuleCard.test.tsx
- frontend/tests/unit/components/ChapterMap.test.tsx
- frontend/tests/unit/pages/Header.test.tsx
Comment on lines
5
to
38
| jest.mock('@heroui/react', () => ({ | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| Input: jest.fn( | ||
| ({ | ||
| id, | ||
| label, | ||
| value, | ||
| onValueChange, | ||
| type, | ||
| isRequired, | ||
| isInvalid, | ||
| errorMessage, | ||
| min, | ||
| max, | ||
| labelPlacement, | ||
| classNames, | ||
| }) => ( | ||
| <div data-testid="mock-input-container"> | ||
| <label htmlFor={id}>{label}</label> | ||
| <input | ||
| id={id} | ||
| type={type} | ||
| value={value} | ||
| onChange={(e) => onValueChange(e.target.value)} | ||
| required={isRequired} | ||
| min={min} | ||
| max={max} | ||
| aria-invalid={isInvalid} | ||
| data-label-placement={labelPlacement} | ||
| data-class-names={JSON.stringify(classNames)} | ||
| /> | ||
| {errorMessage && <span data-testid="error-message">{errorMessage}</span>} | ||
| </div> | ||
| ) | ||
| TextField: ({ children, isRequired, isInvalid, value, onChange }) => ( | ||
| <div data-testid="mock-textfield" data-required={isRequired} data-invalid={isInvalid}> | ||
| {React.Children.map(children, (child) => | ||
| React.isValidElement(child) | ||
| ? React.cloneElement(child as React.ReactElement<Record<string, unknown>>, { | ||
| _tfValue: value, | ||
| _tfOnChange: onChange, | ||
| }) | ||
| : child | ||
| )} | ||
| </div> | ||
| ), | ||
| Label: ({ children, htmlFor, className }) => ( | ||
| <label htmlFor={htmlFor} className={className}> | ||
| {children} | ||
| </label> | ||
| ), | ||
| Input: ({ id, type, min, max, className, _tfValue, _tfOnChange }) => ( | ||
| <input | ||
| id={id} | ||
| type={type} | ||
| min={min} | ||
| max={max} | ||
| className={className} | ||
| value={_tfValue ?? ''} | ||
| onChange={(e) => _tfOnChange?.(e.target.value)} | ||
| /> | ||
| ), | ||
| FieldError: ({ children, className }) => ( | ||
| <span data-testid="error-message" className={className}> | ||
| {children} | ||
| </span> | ||
| ), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mock drops TextField's id, so the duplicate-id defect can't surface in tests. The real TextField spreads id onto its wrapper; this mock ignores it, which is why getByLabelText passes here. Rendering id on the mock wrapper would keep the test honest once the component is fixed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsx`
around lines 5 - 38, Update the mocked TextField in FormDateInput tests to
accept its id prop and render that id on the mock wrapper, matching the real
TextField behavior. Keep the existing value, validation, and child-propagation
behavior unchanged so duplicate IDs can be detected once the component is
corrected.
Comment on lines
+29
to
+51
| ComboBox: Object.assign( | ||
| ({ | ||
| children, | ||
| inputValue, | ||
| onInputChange, | ||
| onSelectionChange, | ||
| isInvalid, | ||
| className, | ||
| }: { | ||
| children?: React.ReactNode | ||
| inputValue?: string | ||
| onInputChange?: (value: string) => void | ||
| onSelectionChange?: (key: React.Key | null) => void | ||
| isInvalid?: boolean | ||
| className?: string | ||
| }) => ( | ||
| <div data-testid="autocomplete"> | ||
| <input | ||
| data-testid="autocomplete-input" | ||
| value={inputValue || ''} | ||
| onChange={(e) => onInputChange?.(e.target.value)} | ||
| /> | ||
| <div data-testid="combobox-children">{children}</div> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Stale autocomplete test IDs for the ComboBox mock. Renaming these to combobox-* (as done for the subcomponents) keeps the mock readable after the Autocomplete→ComboBox migration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/components/ModuleForm.test.tsx` around lines 29 - 51,
The ComboBox mock in ModuleForm.test.tsx still uses stale autocomplete test IDs;
rename autocomplete and autocomplete-input to the corresponding combobox-* IDs,
matching the existing combobox-children naming and updating any references in
the test.
Comment on lines
+13
to
+16
| jest.mock('@heroui/react', () => { | ||
| const ReactActual = jest.requireActual('react') | ||
| return { | ||
| ...jest.requireActual('@heroui/react'), |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
jest.requireActual('@heroui/react') pulls the whole real library into every test in this file. Combined with the new transformIgnorePatterns allowlist this transforms the entire HeroUI tree per run; listing only the components actually needed keeps the suite fast and deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx` around lines 13 - 16,
Update the `@heroui/react` mock in CreateModule.test.tsx to avoid spreading
jest.requireActual('`@heroui/react`'). Mock only the HeroUI components and exports
used by this test, while preserving the existing React-based behavior and
required symbols.
Comment on lines
+174
to
+203
| Item: ({ | ||
| children, | ||
| id, | ||
| textValue, | ||
| _onSelectionChange, | ||
| ...props | ||
| }: { | ||
| children?: React.ReactNode | ||
| id?: string | ||
| textValue?: string | ||
| _onSelectionChange?: (key: string) => void | ||
| [key: string]: unknown | ||
| }) => ( | ||
| <li | ||
| data-testid="listbox-item" | ||
| data-id={id} | ||
| data-key={id} | ||
| role="option" | ||
| aria-selected={false} | ||
| aria-label={textValue} | ||
| tabIndex={0} | ||
| onClick={() => _onSelectionChange?.(id ?? '')} | ||
| onKeyDown={(e) => e.key === 'Enter' && _onSelectionChange?.(id ?? '')} | ||
| {...props} | ||
| > | ||
| {textValue ?? children} | ||
| </li> | ||
| ), | ||
| } | ||
| ), |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Spread {...props} before the handlers. Any prop forwarded through cloneElement (e.g. a future onClick) currently overrides the mock's onClick/onKeyDown, silently breaking option selection in these tests.
♻️ Proposed fix
<li
+ {...props}
data-testid="listbox-item"
data-id={id}
data-key={id}
role="option"
aria-selected={false}
aria-label={textValue}
tabIndex={0}
onClick={() => _onSelectionChange?.(id ?? '')}
onKeyDown={(e) => e.key === 'Enter' && _onSelectionChange?.(id ?? '')}
- {...props}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Item: ({ | |
| children, | |
| id, | |
| textValue, | |
| _onSelectionChange, | |
| ...props | |
| }: { | |
| children?: React.ReactNode | |
| id?: string | |
| textValue?: string | |
| _onSelectionChange?: (key: string) => void | |
| [key: string]: unknown | |
| }) => ( | |
| <li | |
| data-testid="listbox-item" | |
| data-id={id} | |
| data-key={id} | |
| role="option" | |
| aria-selected={false} | |
| aria-label={textValue} | |
| tabIndex={0} | |
| onClick={() => _onSelectionChange?.(id ?? '')} | |
| onKeyDown={(e) => e.key === 'Enter' && _onSelectionChange?.(id ?? '')} | |
| {...props} | |
| > | |
| {textValue ?? children} | |
| </li> | |
| ), | |
| } | |
| ), | |
| Item: ({ | |
| children, | |
| id, | |
| textValue, | |
| _onSelectionChange, | |
| ...props | |
| }: { | |
| children?: React.ReactNode | |
| id?: string | |
| textValue?: string | |
| _onSelectionChange?: (key: string) => void | |
| [key: string]: unknown | |
| }) => ( | |
| <li | |
| {...props} | |
| data-testid="listbox-item" | |
| data-id={id} | |
| data-key={id} | |
| role="option" | |
| aria-selected={false} | |
| aria-label={textValue} | |
| tabIndex={0} | |
| onClick={() => _onSelectionChange?.(id ?? '')} | |
| onKeyDown={(e) => e.key === 'Enter' && _onSelectionChange?.(id ?? '')} | |
| > | |
| {textValue ?? children} | |
| </li> | |
| ), | |
| } | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/pages/CreateModule.test.tsx` around lines 174 - 203,
Update the mocked Item component in CreateModule tests so the {...props} spread
occurs before the explicit onClick and onKeyDown handlers, ensuring forwarded
props cannot override selection behavior while preserving the existing handler
logic.
Comment on lines
+52
to
+56
| Next: ({ children, onPress, isDisabled }) => ( | ||
| <button onClick={() => onPress && onPress()} disabled={isDisabled}> | ||
| Next Page | ||
| </button> | ||
| ), |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Next mock ignores children and hardcodes "Next Page". Unlike Previous, it can't surface what the component actually renders, so a mislabeled next control would still pass. Render {children} and query by role/aria-label instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx` around
lines 52 - 56, Update the Next mock in ProjectsHealthDashboardMetrics tests to
render its children instead of hardcoding “Next Page,” and adjust the related
assertions to query the control by role or aria-label so the test validates the
component’s actual label.
| "build": "next build", | ||
| "dev": "next dev --port 3000 --turbo", | ||
| "graphql-codegen": "graphql-codegen --config graphql-codegen-config.ts", | ||
| "graphql-codegen": "graphql-codegen --config graphql-codegen.ts", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a '^graphql-codegen.*\.ts$' frontend
rg -n "graphql-codegen-config|graphql-codegen\.ts" frontend/package.json frontend --glob '*.ts'Repository: OWASP/Nest
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked file entries:\n'
git ls-files | rg '(^|/)graphql-codegen(-config)?\.ts$' || true
printf '\nPackage script context:\n'
sed -n '90,105p' frontend/package.json
printf '\nConfig file presence:\n'
python3 - <<'PY'
from pathlib import Path
for name in ['frontend/graphql-codegen-config.ts', 'frontend/graphql-codegen.ts']:
p = Path(name)
print(f'{name}: tracked={p in __import__("subprocess").check_output(["git","ls-files"], text=True).splitlines()} exists={p.exists()} size={p.stat().st_size if p.exists() else None}')
PYRepository: OWASP/Nest
Length of output: 1106
Update the codegen script to the tracked config filename.
frontend/package.json points to graphql-codegen.ts, but the tracked codegen config is frontend/graphql-codegen-config.ts. Rename the config file to graphql-codegen.ts or change this script to --config graphql-codegen-config.ts so pnpm graphql-codegen resolves the active configuration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/package.json` at line 99, Update the graphql-codegen script to
reference the tracked configuration filename graphql-codegen-config.ts, or
rename that tracked file to graphql-codegen.ts while keeping the script
consistent. Ensure pnpm graphql-codegen resolves the active configuration.
Comment on lines
33
to
+59
| return ( | ||
| <div className={className || 'w-full min-w-0'} style={{ maxWidth: '100%', overflow: 'hidden' }}> | ||
| <Input | ||
| <TextField | ||
| id={id} | ||
| type={type} | ||
| label={label} | ||
| labelPlacement="outside" | ||
| placeholder={placeholder} | ||
| value={value} | ||
| onValueChange={onValueChange} | ||
| onBlur={onBlur} | ||
| isRequired={required} | ||
| isInvalid={touched && !!error} | ||
| errorMessage={touched ? error : undefined} | ||
| min={min} | ||
| classNames={COMMON_INPUT_CLASS_NAMES} | ||
| /> | ||
| value={value} | ||
| onChange={onValueChange} | ||
| className="w-full min-w-0" | ||
| > | ||
| <Label htmlFor={id} className="text-sm font-semibold text-gray-600 dark:text-gray-300"> | ||
| {label} | ||
| </Label> | ||
| <Input | ||
| id={id} | ||
| type={type} | ||
| placeholder={placeholder} | ||
| onBlur={onBlur} | ||
| min={min} | ||
| className="w-full min-w-0 bg-gray-50 text-gray-800 dark:bg-gray-800 dark:text-gray-200" | ||
| /> | ||
| {touched && error && ( | ||
| <FieldError className="w-full max-w-full text-sm break-words text-red-500"> | ||
| {error} | ||
| </FieldError> | ||
| )} | ||
| </TextField> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Layout regression from dropping COMMON_INPUT_CLASS_NAMES is still open.
The maintainer's screenshot shows the label/input/error stack losing its previous spacing and input chrome. TextField here has no layout classes, so Label, Input, and FieldError render as unstyled siblings. Adding the vertical stack back on the TextField (e.g. flex flex-col gap-1.5) plus the previous input border/rounding restores the intended look. Want me to draft the class set for both FormTextInput and FormDateInput?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/forms/shared/FormTextInput.tsx` around lines 33 - 59,
The FormTextInput TextField layout lost its shared spacing and input styling.
Update the TextField className to restore the vertical stack with appropriate
gap, and update the Input className to include the previous border and rounding
classes while preserving the existing width, background, and text styles; apply
the same restoration in FormDateInput if it uses the corresponding shared
styling.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
|
Contribution validation failed:
|
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
… trigger semantics Signed-off-by: Adarshkumar0509 <adarshkumarthakur0509@gmail.com>
|
Contribution validation failed:
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #5168
Add the PR description here.
Upgrades @heroui/react from v2.8.10 to v3.2.2. HeroUI v3 is built on React Aria Components and introduces breaking API changes across several components.
changes
Checklist