-
Notifications
You must be signed in to change notification settings - Fork 0
[CRT-178] Update lesson list UI #385
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
[CRT-178] Update lesson list UI #385
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 updates the lesson list UI by migrating from PrimeReact DataTable to a custom ChakraDataTable implementation, updating styling and layout, and improving overall user experience.
- Migrated SessionList from PrimeReact to ChakraDataTable with improved column definitions
- Removed deprecated ButtonVariant, InputVariant, and CardVariant type exports in favor of string literals
- Updated navigation tabs to include icons and improved breadcrumb separators
Reviewed Changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/session/SessionList.tsx | Complete migration from PrimeReact DataTable to ChakraDataTable with new column structure and simplified state management |
| frontend/src/components/ChakraDataTable.tsx | Updated onRowClick signature to include mouse event parameter and removed custom styled components in favor of recipe-based styling |
| frontend/src/components/ui/recipes/table/TableRoot.recipe.ts | Added new table recipe for consistent styling across tables |
| frontend/src/components/class/ClassNavigation.tsx | Added icons to navigation tabs and fixed typo in "sessionsTab" message key |
| frontend/src/components/Button.tsx | Removed ButtonVariant export and active prop, simplified to use Chakra's native variant prop |
| frontend/src/components/Breadcrumbs.tsx | Simplified breadcrumb rendering by delegating separator rendering to child components |
| frontend/src/utilities/table.ts | Updated isClickOnRow to accept generic React mouse event instead of PrimeReact-specific event |
| Multiple component files | Updated Button variant props from enum to string literals |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
5 issues found across 32 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="frontend/src/components/Button.tsx">
<violation number="1" location="frontend/src/components/Button.tsx:76">
ChakraButton expects an `isLoading` prop; passing `loading={isLoading}` is ignored, so the Chakra button never reflects the pending state (disabled/spinner/aria) even when the promise is running. Use `isLoading` instead of `loading`.</violation>
</file>
<file name="frontend/src/components/Pagination.tsx">
<violation number="1" location="frontend/src/components/Pagination.tsx:66">
The updated pagination items render every page button with the same outline style, so there is no visual indication of the current page. This regresses usability because the active page styling (`state === "active"`) was removed and `_selected` inside the `variant` prop is never applied.</violation>
</file>
<file name="frontend/src/components/Breadcrumbs.tsx">
<violation number="1" location="frontend/src/components/Breadcrumbs.tsx:19">
Rendering `{children}` directly removed the automatic `<Breadcrumb.Separator />` insertion, so breadcrumb items now appear without separators. Please restore the separator-wrapping logic to keep the breadcrumb UI consistent.</violation>
</file>
<file name="frontend/src/components/ChakraDataTable.tsx">
<violation number="1" location="frontend/src/components/ChakraDataTable.tsx:127">
The last-column styling never applies because the new selector key `"td#{&}:last-of-type"` is invalid CSS in Chakra/Emotion, so the right-alignment and nowrap rules are dropped. Use a valid selector such as `"td&:last-of-type"` or scope via `_last` to restore the formatting.</violation>
</file>
<file name="frontend/src/components/class/ClassList.tsx">
<violation number="1" location="frontend/src/components/class/ClassList.tsx:64">
Rendering the class name with Chakra Text (which outputs a <p>) makes the new isClickOnRow guard think the click was outside the row, so clicking the title no longer navigates to class details.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
577fb32 to
1e7b635
Compare
1e7b635 to
7d41bd2
Compare
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.
No issues found across 32 files
5f788d9
into
feature/CRT-197_ChakraUIIntegration
Application:
Figma:
Summary by cubic
Redesigned the class Lessons list and related UI to match CRT-178 Figma. Switched tables to Chakra, updated copy and icons, and added a “Share” quick action.
New Features
Refactors
This aligns the lesson list with CRT-178’s design. “Copy existing lesson” is intentionally not included.
Written for commit bbfeff3. Summary will update automatically on new commits.