[CRT-197] Chakra UI integration #377
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
15 issues found across 39 files
Prompt for AI agents (all 15 issues)
Understand the root cause of the following 15 issues and fix them.
<file name="frontend/content/locales/en.json">
<violation number="1" location="frontend/content/locales/en.json:153">
Fix the grammar in this label so it reads “Select a solution.”</violation>
</file>
<file name="frontend/src/components/class/ClassForm.tsx">
<violation number="1" location="frontend/src/components/class/ClassForm.tsx:64">
Please pass invalid={!!errors.name} so the Chakra Field marks this input as invalid when a validation error is present; otherwise the new Input never sets the invalid state and the field stays in its default styling despite having an error message.</violation>
</file>
<file name="frontend/src/components/Header.tsx">
<violation number="1" location="frontend/src/components/Header.tsx:105">
Restoring a `data-testid="current-user"` attribute on the dropdown trigger keeps the e2e selector (`headerCurrentUserName`) working; consider wrapping the trigger value so the attribute is preserved.</violation>
</file>
<file name="frontend/src/components/Button.tsx">
<violation number="1" location="frontend/src/components/Button.tsx:126">
Use ChakraButton's isLoading prop. Using loading here is ignored, so the promise-loading spinner/disabled state never activates.</violation>
</file>
<file name="frontend/src/components/class/ClassList.tsx">
<violation number="1" location="frontend/src/components/class/ClassList.tsx:142">
Clicking the Delete menu item now bubbles to the table row, triggering the onRowClick navigation before the confirmation modal can open; please stop propagation in this handler to restore the previous behavior.</violation>
</file>
<file name="frontend/content/compiled-locales/fr.json">
<violation number="1" location="frontend/content/compiled-locales/fr.json:546">
The translation id now contains "DEPRECATED:" twice, so callers looking up "DEPRECATED: Category.filteredOut" will no longer find this string. Please keep the original single-prefix key (and update the other affected entries alongside this one).</violation>
</file>
<file name="frontend/src/components/user/UserList.tsx">
<violation number="1" location="frontend/src/components/user/UserList.tsx:89">
The new dropdown trigger button navigates to the edit page on every click, so the menu never opens and the delete / registration-token actions are no longer accessible.</violation>
</file>
<file name="frontend/content/compiled-locales/en.json">
<violation number="1" location="frontend/content/compiled-locales/en.json:323">
Dropping the trailing space from the group prefix will render labels without spacing, e.g. "Group:My Group".</violation>
</file>
<file name="frontend/src/components/session/SessionList.tsx">
<violation number="1" location="frontend/src/components/session/SessionList.tsx:169">
Wrapping the "create session" button in Dropdown now shows a chevron trigger that opens an empty menu because no menu items are provided; this regresses the prior plain button behavior. Please render the button directly (or supply real menu items) so the control doesn’t advertise a non-existent dropdown.</violation>
</file>
<file name="frontend/src/components/form/TextArea.tsx">
<violation number="1" location="frontend/src/components/form/TextArea.tsx:48">
`Field.Label` renders a `<label>`, but `InputWrapper` is already a `<label>`, so this change creates nested labels, which is invalid HTML and breaks accessibility. Please ensure only one label wraps the textarea.</violation>
</file>
<file name="frontend/src/components/ChakraDataTable.tsx">
<violation number="1" location="frontend/src/components/ChakraDataTable.tsx:44">
The default branch of SortIcon renders a descending arrow even when the column is unsorted, so headers with no active sort still appear sorted descending. Render nothing when isSorted is false to avoid misleading users.</violation>
</file>
<file name="frontend/src/components/form/ChakraRange.tsx">
<violation number="1" location="frontend/src/components/form/ChakraRange.tsx:33">
Chakra UI’s slider does not expose `.Root`, `.Control`, `.Track`, `.Range`, or `.Thumbs`, nor `onValueChange(details)` handlers. Rendering this will try to mount undefined components and the range slider will crash; swap to Chakra’s `<RangeSlider>` structure and supported `onChange`/`onChangeEnd` callbacks.</violation>
</file>
<file name="frontend/src/components/task/TaskTable.tsx">
<violation number="1" location="frontend/src/components/task/TaskTable.tsx:99">
Clicking the dropdown chevron still bubbles to the row’s onRowClick, so the router navigates away before the menu opens and the delete option is unreachable. Please stop propagation when toggling this Dropdown.</violation>
</file>
<file name="frontend/src/components/form/Input.tsx">
<violation number="1" location="frontend/src/components/form/Input.tsx:52">
Rendering Field.Label here nests a <label> inside InputWrapper (which is itself a label), creating invalid HTML that breaks the implicit label/input association. Please wrap Field.Root in a non-label element or convert InputWrapper to a div before using Field.Label.</violation>
</file>
<file name="frontend/src/components/form/SortableList.tsx">
<violation number="1" location="frontend/src/components/form/SortableList.tsx:118">
Wrapping each sortable item in Listbox.ItemText makes the nested form controls (inputs/remove buttons rendered by callers) non-interactive because ItemText disables pointer events, so editing or removing entries no longer works. Remove the ItemText wrapper or restructure the layout so interactive controls are outside it.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
Ticket analysis
Linked issue: CRT-197: [UI] Integrate ChakraUI in project
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add ChakraProvider and wrap app with provider | Provider added and _app.tsx wrapped with |
| ✅ | Add @chakra-ui/react and related dependencies (react-icons, @tanstack/react-table, next-themes) | Dependencies added to package.json |
| ✅ | Enable Next.js optimizePackageImports for Chakra | next.config.ts experimental optimizePackageImports set |
| ✅ | Migrate Button to Chakra with async loading/success/failure state | Button uses ChakraButton, loading and success/failure handling |
| ✅ | Migrate Input to Chakra and support errorText display | Input now uses Chakra Field/Input and errorText prop |
| ✅ | Migrate TextArea to Chakra and support errorText display | TextArea uses Chakra Field/Textarea and errorText prop |
| ✅ | Replace PrimeRange with Chakra Slider and add ChakraRange component | ChakraRange component added and Range/MinMaxRange use it |
| ✅ | Migrate SortableList to Chakra Listbox integration | SortableList uses Chakra createListCollection and Listbox |
| ✅ | Migrate Tag to Chakra Tag component | Tag now renders ChakraTag with deterministic palette |
| ✅ | Use Chakra CloseButton where applicable | CloseButton imported from Chakra and used in Task |
| ✅ | Introduce custom Dropdown implemented with Chakra Menu | New Dropdown component using Chakra Menu API added |
| ✅ | Introduce ChakraDataTable using TanStack Table with sorting/filtering/pagination | ChakraDataTable implements tanstack hooks and pagination/filters/sorting |
| ✅ | Update Header and LanguageChooser to use new Dropdown | Header and LanguageChooser replaced react-bootstrap dropdowns with Dropdown |
| ✅ | Update Class/Lesson/Session/User lists to use new Dropdown/Button | List components replaced ButtonGroup/Dropdown with Dropdown + Button triggers |
| ✅ | Locale updates for datatable labels, placeholders and task/reference-solution fields, and simplified criteria labels | Locale files updated with datatable keys and criteria/message changes |
| ✅ | Track scratch move/remove events for blocks in large stacks | should-track-move now checks wasPartOfLargeStack(event) |
| ✅ | Remove PrimeRange and react-bootstrap Dropdown usage project-wide | PrimeRange removed and many react-bootstrap Dropdown imports replaced |
| ✅ | TS config updates: moduleResolution Bundler and target esnext | tsconfig.json changed moduleResolution and target |
frontend/content/locales/en.json
Outdated
| }, | ||
| "CodeComparison.defaultSolutionOption": { | ||
| "message": "Select a solution" | ||
| "message": "Select a solutions" |
There was a problem hiding this comment.
Fix the grammar in this label so it reads “Select a solution.”
Prompt for AI agents
Address the following comment on frontend/content/locales/en.json at line 153:
<comment>Fix the grammar in this label so it reads “Select a solution.”</comment>
<file context>
@@ -147,10 +150,10 @@
},
"CodeComparison.defaultSolutionOption": {
- "message": "Select a solution"
+ "message": "Select a solutions"
},
"CodeComparison.groupPrefix": {
</file context>
| "message": "Select a solutions" | |
| "message": "Select a solution" |
✅ Addressed in 110ec24
| label={messages.name} | ||
| {...register("name")} | ||
| data-testid="name" | ||
| errorText={errors.name?.message} |
There was a problem hiding this comment.
Please pass invalid={!!errors.name} so the Chakra Field marks this input as invalid when a validation error is present; otherwise the new Input never sets the invalid state and the field stays in its default styling despite having an error message.
Prompt for AI agents
Address the following comment on frontend/src/components/class/ClassForm.tsx at line 64:
<comment>Please pass invalid={!!errors.name} so the Chakra Field marks this input as invalid when a validation error is present; otherwise the new Input never sets the invalid state and the field stays in its default styling despite having an error message.</comment>
<file context>
@@ -57,11 +57,12 @@ const ClassForm = ({
+ label={messages.name}
+ {...register("name")}
+ data-testid="name"
+ errorText={errors.name?.message}
+ ></Input>
</file context>
✅ Addressed in ba473f8
There was a problem hiding this comment.
Without retrieving the invalid state from the useForm above, there's indeed no way of knowing if the error text should be displayed or not.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
frontend/src/components/Header.tsx
Outdated
| /> | ||
| </DropdownLinkItem> | ||
| </Dropdown.Menu> | ||
| <Dropdown trigger={name}> |
There was a problem hiding this comment.
Restoring a data-testid="current-user" attribute on the dropdown trigger keeps the e2e selector (headerCurrentUserName) working; consider wrapping the trigger value so the attribute is preserved.
Prompt for AI agents
Address the following comment on frontend/src/components/Header.tsx at line 105:
<comment>Restoring a `data-testid="current-user"` attribute on the dropdown trigger keeps the e2e selector (`headerCurrentUserName`) working; consider wrapping the trigger value so the attribute is preserved.</comment>
<file context>
@@ -102,19 +102,13 @@ const Header = ({
- />
- </DropdownLinkItem>
- </Dropdown.Menu>
+ <Dropdown trigger={name}>
+ <DropdownItem href="/logout">
+ <FormattedMessage
</file context>
| <Dropdown trigger={name}> | |
| <Dropdown trigger={<span data-testid="current-user">{name}</span>}> |
✅ Addressed in 32d4597
| } | ||
| > | ||
| <DropdownItem | ||
| onClick={() => { |
There was a problem hiding this comment.
Clicking the Delete menu item now bubbles to the table row, triggering the onRowClick navigation before the confirmation modal can open; please stop propagation in this handler to restore the previous behavior.
Prompt for AI agents
Address the following comment on frontend/src/components/class/ClassList.tsx at line 142:
<comment>Clicking the Delete menu item now bubbles to the table row, triggering the onRowClick navigation before the confirmation modal can open; please stop propagation in this handler to restore the previous behavior.</comment>
<file context>
@@ -124,37 +124,29 @@ const ClassList = () => {
+ }
+ >
+ <DropdownItem
+ onClick={() => {
+ setClassIdToDelete(rowData.id);
+ setShowDeleteConfirmationModal(true);
</file context>
✅ Addressed in 32d4597
There was a problem hiding this comment.
Have you checked for this @bubudetp? It seems plausible that this would be an issue.
There was a problem hiding this comment.
Yeah, the bot was right, just updated this.
| return <LuArrowUp size={16} />; | ||
| } | ||
|
|
||
| return <LuArrowDown size={16} />; |
There was a problem hiding this comment.
The default branch of SortIcon renders a descending arrow even when the column is unsorted, so headers with no active sort still appear sorted descending. Render nothing when isSorted is false to avoid misleading users.
Prompt for AI agents
Address the following comment on frontend/src/components/ChakraDataTable.tsx at line 44:
<comment>The default branch of SortIcon renders a descending arrow even when the column is unsorted, so headers with no active sort still appear sorted descending. Render nothing when isSorted is false to avoid misleading users.</comment>
<file context>
@@ -0,0 +1,567 @@
+ return <LuArrowUp size={16} />;
+ }
+
+ return <LuArrowDown size={16} />;
+};
+
</file context>
| return <LuArrowDown size={16} />; | |
| return isSorted === "desc" ? <LuArrowDown size={16} /> : null; |
✅ Addressed in 5edb027
There was a problem hiding this comment.
This is the intended design, showing the arrowup icon when the filter is not applied and vice versa
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
|
|
||
| <Dropdown.Menu> | ||
| <Dropdown.Item | ||
| <Dropdown |
There was a problem hiding this comment.
Clicking the dropdown chevron still bubbles to the row’s onRowClick, so the router navigates away before the menu opens and the delete option is unreachable. Please stop propagation when toggling this Dropdown.
Prompt for AI agents
Address the following comment on frontend/src/components/task/TaskTable.tsx at line 99:
<comment>Clicking the dropdown chevron still bubbles to the row’s onRowClick, so the router navigates away before the menu opens and the delete option is unreachable. Please stop propagation when toggling this Dropdown.</comment>
<file context>
@@ -96,38 +96,29 @@ const TaskTable = () => {
-
- <Dropdown.Menu>
- <Dropdown.Item
+ <Dropdown
+ trigger={
+ <Button
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
This has been adressed by moving the stop propagation inside the dropdown component
There was a problem hiding this comment.
Thanks for the feedback! I've updated an existing learning with this new information.
| <StyledInput {...inputProps} ref={ref} /> | ||
| {children} | ||
| <Field.Root invalid={invalid}> | ||
| {label && <Field.Label>{intl.formatMessage(label)}</Field.Label>} |
There was a problem hiding this comment.
Rendering Field.Label here nests a inside InputWrapper (which is itself a label), creating invalid HTML that breaks the implicit label/input association. Please wrap Field.Root in a non-label element or convert InputWrapper to a div before using Field.Label.
Prompt for AI agents
Address the following comment on frontend/src/components/form/Input.tsx at line 52:
<comment>Rendering Field.Label here nests a <label> inside InputWrapper (which is itself a label), creating invalid HTML that breaks the implicit label/input association. Please wrap Field.Root in a non-label element or convert InputWrapper to a div before using Field.Label.</comment>
<file context>
@@ -1,40 +1,67 @@
- <StyledInput {...inputProps} ref={ref} />
- {children}
+ <Field.Root invalid={invalid}>
+ {label && <Field.Label>{intl.formatMessage(label)}</Field.Label>}
+ <ChakraInput
+ css={styledInputStyles}
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
This just requires changing the InputWrapper HTML tag type. The problem was there before.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
There was a problem hiding this comment.
@bubudetp I think this one still needs handling.
| testId={`${testId}-item-${collectionItem.originalItem.id}`} | ||
| item={collectionItem} | ||
| > | ||
| <Listbox.ItemText> |
There was a problem hiding this comment.
Wrapping each sortable item in Listbox.ItemText makes the nested form controls (inputs/remove buttons rendered by callers) non-interactive because ItemText disables pointer events, so editing or removing entries no longer works. Remove the ItemText wrapper or restructure the layout so interactive controls are outside it.
Prompt for AI agents
Address the following comment on frontend/src/components/form/SortableList.tsx at line 118:
<comment>Wrapping each sortable item in Listbox.ItemText makes the nested form controls (inputs/remove buttons rendered by callers) non-interactive because ItemText disables pointer events, so editing or removing entries no longer works. Remove the ItemText wrapper or restructure the layout so interactive controls are outside it.</comment>
<file context>
@@ -87,17 +106,25 @@ const SortableListInput = <TItem extends { id: number }>({
+ testId={`${testId}-item-${collectionItem.originalItem.id}`}
+ item={collectionItem}
+ >
+ <Listbox.ItemText>
+ {renderItemContent(
+ collectionItem.originalItem,
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
cubic analysis
18 issues found across 38 files
Prompt for AI agents (all 18 issues)
Understand the root cause of the following 18 issues and fix them.
<file name="frontend/package.json">
<violation number="1" location="frontend/package.json:45">
Adding Chakra UI requires also adding its peer dependency `framer-motion`. Without it, Chakra components that rely on animations will fail to resolve the module at build/runtime. Please add `framer-motion` alongside this dependency.</violation>
</file>
<file name="frontend/src/components/ChakraDataTable.tsx">
<violation number="1" location="frontend/src/components/ChakraDataTable.tsx:44">
The unsorted state still renders the descending arrow, so users cannot tell the difference between unsorted and descending columns. Please return no icon (or a neutral indicator) when the column is unsorted so the sort indicator stays accurate.</violation>
</file>
<file name="frontend/src/components/user/UserForm.tsx">
<violation number="1" location="frontend/src/components/user/UserForm.tsx:81">
When a validation error occurs this Input still renders without the Chakra `invalid` state, so users lose the expected error styling and aria-invalid flag. Please pass `invalid={!!errors.name}` together with the error text.</violation>
<violation number="2" location="frontend/src/components/user/UserForm.tsx:88">
The email field now omits the Chakra `invalid` prop, so validation errors will not flag the control as invalid for styling or accessibility. Pass `invalid={!!errors.email}` when providing the error text.</violation>
</file>
<file name="frontend/content/compiled-locales/fr.json">
<violation number="1" location="frontend/content/compiled-locales/fr.json:546">
The translation key now includes the "DEPRECATED: " prefix twice, changing the identifier and breaking any remaining lookups that still rely on the single-prefixed key. Please keep only one "DEPRECATED: " prefix so the key stays compatible.</violation>
</file>
<file name="frontend/src/components/form/TextArea.tsx">
<violation number="1" location="frontend/src/components/form/TextArea.tsx:48">
Rendering `<Field.Label>` inside `InputWrapper` results in a `<label>` nested inside another `<label>`, producing invalid HTML and breaking accessibility for the textarea.</violation>
</file>
<file name="frontend/src/components/class/ClassForm.tsx">
<violation number="1" location="frontend/src/components/class/ClassForm.tsx:64">
Pass the `invalid` prop when the field has a validation error so the Chakra Field applies error state styling and accessibility attributes.</violation>
</file>
<file name="frontend/src/components/lesson/LessonList.tsx">
<violation number="1" location="frontend/src/components/lesson/LessonList.tsx:138">
Passing our Button component directly to Dropdown.trigger nests a `<button>` inside Chakra's `<Menu.Trigger>` button, producing invalid markup and unreliable menu behavior. Please adjust the trigger so only one button element is rendered (e.g., update Dropdown to use `Menu.Trigger`'s `asChild` prop or pass a non-button wrapper).</violation>
</file>
<file name="frontend/src/components/class/ClassList.tsx">
<violation number="1" location="frontend/src/components/class/ClassList.tsx:127">
Clicking the dropdown chevron now triggers the row’s navigation handler before the menu can open, so the Delete action becomes unreachable after this refactor.</violation>
</file>
<file name="frontend/src/components/form/Input.tsx">
<violation number="1" location="frontend/src/components/form/Input.tsx:53">
Nested content passed to `<Input>` is no longer rendered because it gets forwarded into `<ChakraInput>` instead of being output after the field, so existing validation/error helpers silently vanish.</violation>
</file>
<file name="frontend/src/components/user/UserList.tsx">
<violation number="1" location="frontend/src/components/user/UserList.tsx:173">
Passing the Button component as the dropdown trigger nests a `<button>` inside the `<Menu.Trigger>` button, yielding invalid HTML that breaks accessibility/keyboard support. Please update the dropdown trigger so it doesn’t render a button inside another button (e.g., adjust Dropdown to forward `asChild` or pass a non-button trigger node).</violation>
</file>
<file name="frontend/src/components/Button.tsx">
<violation number="1" location="frontend/src/components/Button.tsx:126">
`ChakraButton` expects the boolean prop `isLoading`, so `loading={isLoading}` is ignored and the spinner never shows. Replace it with `isLoading={isLoading}` to preserve the button’s loading state.</violation>
</file>
<file name="frontend/content/compiled-locales/en.json">
<violation number="1" location="frontend/content/compiled-locales/en.json:317">
The updated locale string contains a grammatical error; please change it to "Select a solution".</violation>
<violation number="2" location="frontend/content/compiled-locales/en.json:323">
Restoring the trailing space keeps the rendered group labels legible when concatenated with the group name.</violation>
</file>
<file name="frontend/content/locales/en.json">
<violation number="1" location="frontend/content/locales/en.json:153">
Fix the user-facing copy so the default solution option reads grammatically correctly.</violation>
<violation number="2" location="frontend/content/locales/en.json:156">
Restore the trailing space in the group prefix label so concatenated group names remain properly spaced.</violation>
</file>
<file name="frontend/src/components/task/TaskTable.tsx">
<violation number="1" location="frontend/src/components/task/TaskTable.tsx:99">
The create button is now nested inside the dropdown trigger, so the focusable control is Menu.Trigger rather than the inner Button. Enter/Space opens an empty menu instead of calling router.push("task/create"), which removes keyboard access to the create action. Please render the create button outside the dropdown or adjust the dropdown to render the trigger as the actual button element.</violation>
<violation number="2" location="frontend/src/components/task/TaskTable.tsx:100">
Wrapping the edit <Button> inside the new <Dropdown> trigger breaks keyboard access: the trigger itself is the only focusable button now, so pressing Enter/Space just opens the menu and never runs the nested button’s router.push. Keyboard users can no longer reach the edit action. Please keep the edit button separate from the dropdown or move the edit handling into the menu itself.</violation>
</file>
Linked issue analysis
Linked issue: CRT-197: [UI] Integrate ChakraUI in project
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add ChakraProvider and wrap app with provider | Provider added and _app.tsx wrapped with |
| ✅ | Add @chakra-ui/react and related dependencies (react-icons, @tanstack/react-table, next-themes) | Dependencies added to package.json |
| ✅ | Enable Next.js optimizePackageImports for Chakra | next.config.ts experimental optimizePackageImports set |
| ✅ | Migrate Button to Chakra with async loading/success/failure state | Button uses ChakraButton, loading and success/failure handling |
| ✅ | Migrate Input to Chakra and support errorText display | Input now uses Chakra Field/Input and errorText prop |
| ✅ | Migrate TextArea to Chakra and support errorText display | TextArea uses Chakra Field/Textarea and errorText prop |
| ✅ | Replace PrimeRange with Chakra Slider and add ChakraRange component | ChakraRange component added and Range/MinMaxRange use it |
| ✅ | Migrate SortableList to Chakra Listbox integration | SortableList uses Chakra createListCollection and Listbox |
| ✅ | Migrate Tag to Chakra Tag component | Tag now renders ChakraTag with deterministic palette |
| ✅ | Use Chakra CloseButton where applicable | CloseButton imported from Chakra and used in Task |
| ✅ | Introduce custom Dropdown implemented with Chakra Menu | New Dropdown component using Chakra Menu API added |
| ✅ | Introduce ChakraDataTable using TanStack Table with sorting/filtering/pagination | ChakraDataTable implements tanstack hooks and pagination/filters/sorting |
| ✅ | Update Header and LanguageChooser to use new Dropdown | Header and LanguageChooser replaced react-bootstrap dropdowns with Dropdown |
| ✅ | Update Class/Lesson/Session/User lists to use new Dropdown/Button | List components replaced ButtonGroup/Dropdown with Dropdown + Button triggers |
| ✅ | Locale updates for datatable labels, placeholders and task/reference-solution fields, and simplified criteria labels | Locale files updated with datatable keys and criteria/message changes |
| ✅ | Track scratch move/remove events for blocks in large stacks | should-track-move now checks wasPartOfLargeStack(event) |
| ✅ | Remove PrimeRange and react-bootstrap Dropdown usage project-wide | PrimeRange removed and many react-bootstrap Dropdown imports replaced |
| ✅ | TS config updates: moduleResolution Bundler and target esnext | tsconfig.json changed moduleResolution and target |
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| ] | ||
| }, | ||
| "dependencies": { | ||
| "@chakra-ui/react": "^3.28.0", |
There was a problem hiding this comment.
Adding Chakra UI requires also adding its peer dependency framer-motion. Without it, Chakra components that rely on animations will fail to resolve the module at build/runtime. Please add framer-motion alongside this dependency.
Prompt for AI agents
Address the following comment on frontend/package.json at line 45:
<comment>Adding Chakra UI requires also adding its peer dependency `framer-motion`. Without it, Chakra components that rely on animations will fail to resolve the module at build/runtime. Please add `framer-motion` alongside this dependency.</comment>
<file context>
@@ -42,6 +42,7 @@
]
},
"dependencies": {
+ "@chakra-ui/react": "^3.28.0",
"@dnd-kit/core": "^6.3.1",
"@dnd-kit/sortable": "^10.0.0",
</file context>
| return <LuArrowUp size={16} />; | ||
| } | ||
|
|
||
| return <LuArrowDown size={16} />; |
There was a problem hiding this comment.
The unsorted state still renders the descending arrow, so users cannot tell the difference between unsorted and descending columns. Please return no icon (or a neutral indicator) when the column is unsorted so the sort indicator stays accurate.
Prompt for AI agents
Address the following comment on frontend/src/components/ChakraDataTable.tsx at line 44:
<comment>The unsorted state still renders the descending arrow, so users cannot tell the difference between unsorted and descending columns. Please return no icon (or a neutral indicator) when the column is unsorted so the sort indicator stays accurate.</comment>
<file context>
@@ -0,0 +1,567 @@
+ return <LuArrowUp size={16} />;
+ }
+
+ return <LuArrowDown size={16} />;
+};
+
</file context>
| <Input | ||
| label={messages.email} | ||
| {...register("email")} | ||
| data-testid="email" |
There was a problem hiding this comment.
The email field now omits the Chakra invalid prop, so validation errors will not flag the control as invalid for styling or accessibility. Pass invalid={!!errors.email} when providing the error text.
Prompt for AI agents
Address the following comment on frontend/src/components/user/UserForm.tsx at line 88:
<comment>The email field now omits the Chakra `invalid` prop, so validation errors will not flag the control as invalid for styling or accessibility. Pass `invalid={!!errors.email}` when providing the error text.</comment>
<file context>
@@ -75,13 +75,19 @@ const UserForm = ({
+ <Input
+ label={messages.email}
+ {...register("email")}
+ data-testid="email"
+ errorText={errors.email?.message}
+ ></Input>
</file context>
| data-testid="email" | |
| data-testid="email" invalid={!!errors.email} |
✅ Addressed in ba473f8
| <Input | ||
| label={messages.name} | ||
| {...register("name")} | ||
| data-testid="name" |
There was a problem hiding this comment.
When a validation error occurs this Input still renders without the Chakra invalid state, so users lose the expected error styling and aria-invalid flag. Please pass invalid={!!errors.name} together with the error text.
Prompt for AI agents
Address the following comment on frontend/src/components/user/UserForm.tsx at line 81:
<comment>When a validation error occurs this Input still renders without the Chakra `invalid` state, so users lose the expected error styling and aria-invalid flag. Please pass `invalid={!!errors.name}` together with the error text.</comment>
<file context>
@@ -75,13 +75,19 @@ const UserForm = ({
+ <Input
+ label={messages.name}
+ {...register("name")}
+ data-testid="name"
+ errorText={errors.name?.message}
+ ></Input>
</file context>
| data-testid="name" | |
| data-testid="name" invalid={!!errors.name} |
✅ Addressed in ba473f8
| "value": "Selected" | ||
| } | ||
| ], | ||
| "DEPRECATED: DEPRECATED: Category.filteredOut": [ |
There was a problem hiding this comment.
The translation key now includes the "DEPRECATED: " prefix twice, changing the identifier and breaking any remaining lookups that still rely on the single-prefixed key. Please keep only one "DEPRECATED: " prefix so the key stays compatible.
Prompt for AI agents
Address the following comment on frontend/content/compiled-locales/fr.json at line 546:
<comment>The translation key now includes the "DEPRECATED: " prefix twice, changing the identifier and breaking any remaining lookups that still rely on the single-prefixed key. Please keep only one "DEPRECATED: " prefix so the key stays compatible.</comment>
<file context>
@@ -531,36 +537,48 @@
+ "value": "Selected"
+ }
+ ],
+ "DEPRECATED: DEPRECATED: Category.filteredOut": [
{
"type": 0,
</file context>
| "DEPRECATED: DEPRECATED: Category.filteredOut": [ | |
| "DEPRECATED: Category.filteredOut": [ |
| { | ||
| "type": 0, | ||
| "value": "Select a solution" | ||
| "value": "Select a solutions" |
There was a problem hiding this comment.
The updated locale string contains a grammatical error; please change it to "Select a solution".
Prompt for AI agents
Address the following comment on frontend/content/compiled-locales/en.json at line 317:
<comment>The updated locale string contains a grammatical error; please change it to "Select a solution".</comment>
<file context>
@@ -308,13 +314,13 @@
{
"type": 0,
- "value": "Select a solution"
+ "value": "Select a solutions"
}
],
</file context>
| "value": "Select a solutions" | |
| "value": "Select a solution" |
✅ Addressed in eeee72e
frontend/content/locales/en.json
Outdated
| }, | ||
| "CodeComparison.defaultSolutionOption": { | ||
| "message": "Select a solution" | ||
| "message": "Select a solutions" |
There was a problem hiding this comment.
Fix the user-facing copy so the default solution option reads grammatically correctly.
Prompt for AI agents
Address the following comment on frontend/content/locales/en.json at line 153:
<comment>Fix the user-facing copy so the default solution option reads grammatically correctly.</comment>
<file context>
@@ -147,10 +150,10 @@
},
"CodeComparison.defaultSolutionOption": {
- "message": "Select a solution"
+ "message": "Select a solutions"
},
"CodeComparison.groupPrefix": {
</file context>
| "message": "Select a solutions" | |
| "message": "Select a solution" |
| }, | ||
| "CodeComparison.groupPrefix": { | ||
| "message": "Group: " | ||
| "message": "Group:" |
There was a problem hiding this comment.
Restore the trailing space in the group prefix label so concatenated group names remain properly spaced.
Prompt for AI agents
Address the following comment on frontend/content/locales/en.json at line 156:
<comment>Restore the trailing space in the group prefix label so concatenated group names remain properly spaced.</comment>
<file context>
@@ -147,10 +150,10 @@
},
"CodeComparison.groupPrefix": {
- "message": "Group: "
+ "message": "Group:"
},
"CodeComparison.heading": {
</file context>
| "message": "Group:" | |
| "message": "Group: " |
|
|
||
| <Dropdown.Menu> | ||
| <Dropdown.Item | ||
| <Dropdown |
There was a problem hiding this comment.
The create button is now nested inside the dropdown trigger, so the focusable control is Menu.Trigger rather than the inner Button. Enter/Space opens an empty menu instead of calling router.push("task/create"), which removes keyboard access to the create action. Please render the create button outside the dropdown or adjust the dropdown to render the trigger as the actual button element.
Prompt for AI agents
Address the following comment on frontend/src/components/task/TaskTable.tsx at line 99:
<comment>The create button is now nested inside the dropdown trigger, so the focusable control is Menu.Trigger rather than the inner Button. Enter/Space opens an empty menu instead of calling router.push("task/create"), which removes keyboard access to the create action. Please render the create button outside the dropdown or adjust the dropdown to render the trigger as the actual button element.</comment>
<file context>
@@ -96,38 +96,29 @@ const TaskTable = () => {
-
- <Dropdown.Menu>
- <Dropdown.Item
+ <Dropdown
+ trigger={
+ <Button
</file context>
✅ Addressed in 2d70c0c
| <Dropdown.Menu> | ||
| <Dropdown.Item | ||
| <Dropdown | ||
| trigger={ |
There was a problem hiding this comment.
Wrapping the edit inside the new trigger breaks keyboard access: the trigger itself is the only focusable button now, so pressing Enter/Space just opens the menu and never runs the nested button’s router.push. Keyboard users can no longer reach the edit action. Please keep the edit button separate from the dropdown or move the edit handling into the menu itself.
Prompt for AI agents
Address the following comment on frontend/src/components/task/TaskTable.tsx at line 100:
<comment>Wrapping the edit <Button> inside the new <Dropdown> trigger breaks keyboard access: the trigger itself is the only focusable button now, so pressing Enter/Space just opens the menu and never runs the nested button’s router.push. Keyboard users can no longer reach the edit action. Please keep the edit button separate from the dropdown or move the edit handling into the menu itself.</comment>
<file context>
@@ -96,38 +96,29 @@ const TaskTable = () => {
- <Dropdown.Menu>
- <Dropdown.Item
+ <Dropdown
+ trigger={
+ <Button
+ variant={ButtonVariant.secondary}
</file context>
✅ Addressed in 2d70c0c
| { | ||
| "type": 0, | ||
| "value": "Select a solution" | ||
| "value": "Select a solutions" |
There was a problem hiding this comment.
It seems a typo was introduced here
| "value": "Select a solutions" | |
| "value": "Select a solution" |
There was a problem hiding this comment.
I think it was there before me because I didn't touch that bit of code. But I'll fix this.
| "datatable.placeholder.filterBy": [ | ||
| { | ||
| "type": 0, | ||
| "value": "Search..." |
There was a problem hiding this comment.
I assume this shouldn't be Search ?
| "JoinSession.anonymousSessionDescription": [ | ||
| { | ||
| "type": 0, | ||
| "value": "This session is anonymous which means you do not need to sign-in. Note that you cannot continue working on the session after closing the browser." | ||
| } | ||
| ], |
There was a problem hiding this comment.
Somehow I doubt this should be removed? 🤔
There was a problem hiding this comment.
I don't know how this got removed, all I did was run the i18n:update command to generate the translations but many things have been introduced. I have not touched this file, so this seems odd.
| } | ||
| ], | ||
| "DEPRECATED: FunctionCallCriterionFilterForm.count": [ | ||
| "DEPRECATED: DEPRECATED: FunctionCallCriterionFilterForm.count": [ |
There was a problem hiding this comment.
how come DEPRECATED: twice?
There was a problem hiding this comment.
Again all I did was run the i8n:update command but it seems like it fails, somehow.
There was a problem hiding this comment.
This seems to come from here:
collimator/frontend/scripts/fill-locales.mjs
Lines 27 to 30 in 4b72f45
| "JoinSession.anonymousSessionDescription": { | ||
| "message": "This session is anonymous which means you do not need to sign-in. Note that you cannot continue working on the session after closing the browser." | ||
| }, |
There was a problem hiding this comment.
I don't even have this message on my local project
| <Button | ||
| variant={ButtonVariant.secondary} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); |
There was a problem hiding this comment.
be careful, I'm not sure you want to drop e.stopPropagation()
| <Dropdown.Toggle | ||
| variant="secondary" | ||
| split | ||
| data-testid={`task-${rowData.id}-actions-dropdown-button`} |
There was a problem hiding this comment.
where did this data-testid go? Have the tests been updated to match this? :-)
There was a problem hiding this comment.
it seems like I didnt commit this for some reason, I usually use the git add -p for the commits, sorry ^^
|
|
||
| const actionsTemplate = useCallback( | ||
| (rowData: ExistingUser) => ( | ||
| <div data-testid={`user-${rowData.id}-actions`}> |
There was a problem hiding this comment.
it seems we've lost another data-testid here, which will likely break tests.
| trigger={ | ||
| <Button |
There was a problem hiding this comment.
This wasn't the trigger, it was its own button, separate from the dropdown. Look at this:
https://react-bootstrap.netlify.app/docs/components/dropdowns/#split-button-dropdowns
Consider : https://chatgpt.com/share/690a5224-f828-8012-9712-d47d52a481f3
There was a problem hiding this comment.
This file I haven't yet reviewed, too many other points that need fixing.
There was a problem hiding this comment.
cubic analysis
Reviewed changes from recent commits (found 9 issues).
9 issues found across 34 files
Prompt for AI agents (all 9 issues)
Understand the root cause of the following 9 issues and fix them.
<file name="frontend/src/components/lesson/LessonList.tsx">
<violation number="1" location="frontend/src/components/lesson/LessonList.tsx:157">
Clicking the Edit button now bubbles up to the row and triggers `onRowClick`, so the detail redirect overrides the edit navigation. Stop propagation before pushing the edit route.</violation>
</file>
<file name="frontend/src/components/class/ClassList.tsx">
<violation number="1" location="frontend/src/components/class/ClassList.tsx:131">
Removing event suppression here causes the edit button to bubble up to the row’s onRowClick, so the edit navigation is immediately overridden by the detail navigation. Please keep the trigger from bubbling (e.g., by stopping propagation in the trigger or the DropdownMenu component) so the edit action remains usable.</violation>
</file>
<file name="frontend/src/components/Breadcrumbs.tsx">
<violation number="1" location="frontend/src/components/Breadcrumbs.tsx:25">
Using child.key directly here causes multiple fragments to share the key null whenever breadcrumb children were authored without explicit keys, producing React's missing-key warning. Provide a fallback (e.g., the Children.map index) when the child has no key.</violation>
</file>
<file name="frontend/src/components/task/TaskTable.tsx">
<violation number="1" location="frontend/src/components/task/TaskTable.tsx:109">
The edit action button now renders with no icon or label, so the edit affordance disappears while the icon placed outside is non-interactive. Please move the icon back inside the button (or render an IconButton) so the edit action remains visible and clickable.</violation>
</file>
<file name="frontend/src/components/ChakraDataTable.stories.tsx">
<violation number="1" location="frontend/src/components/ChakraDataTable.stories.tsx:77">
The status column is reading firstTask instead of status, so the icon will not reflect the actual status value. Please change this to use the status field.</violation>
</file>
<file name="frontend/src/components/session/SessionList.tsx">
<violation number="1" location="frontend/src/components/session/SessionList.tsx:173">
Removing the `stopPropagation` on the edit trigger means the click now bubbles up to the table row’s `onRowClick`, so the user is sent to the session progress view instead of the edit screen. Please keep the event from bubbling (ideally inside `DropdownMenu`’s trigger) before removing it from this call site.
(Based on your team's feedback about centralizing click event suppression inside dropdown components.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/components/form/ChakraRange.tsx">
<violation number="1" location="frontend/src/components/form/ChakraRange.tsx:49">
Please forward the localized label to ChakraRange so the underlying slider exposes an accessible name (e.g., via `Slider.Label`); right now assistive technologies perceive the slider as unlabeled.</violation>
</file>
<file name="frontend/src/components/user/UserList.tsx">
<violation number="1" location="frontend/src/components/user/UserList.tsx:89">
Dropping `e.stopPropagation()` here lets the table row click handler run, so clicking Edit redirects to the detail page instead of staying on the edit route. Please keep the propagation guard when invoking the router.
(Based on your team's feedback about centralizing click suppression inside Dropdown components—this trigger still needs the guard until DropdownMenu handles it.) [FEEDBACK_USED]</violation>
<violation number="2" location="frontend/src/components/user/UserList.tsx:97">
`DropdownMenu` expects a `testId` prop, so this `data-testid` attribute is ignored and no test hook is emitted. Please pass `testId` instead so automated tests can find the element.</violation>
</file>
Linked issue analysis
Linked issue: CRT-197: [UI] Integrate ChakraUI in project
| Status | Acceptance criteria | Notes |
|---|---|---|
| Add ChakraProvider and integrate it into the app (wrap app & storybook) | ChakraProvider component and Storybook decorator added | |
| ✅ | Migrate Button component to Chakra (including async state handling) | Button.tsx rewritten to use ChakraButton and icons |
| ✅ | Update Input component to support invalid/errorText props | Input supports invalid and errorText props now |
| ✅ | Update TextArea component to support invalid/errorText props | TextArea supports invalid and errorText props now |
| ✅ | Replace Range / MinMaxRange with Chakra Slider (ChakraRange) | ChakraRange added and PrimeRange removed; MinMaxRange now uses ChakraRange |
| ✅ | Replace SortableList implementation to use Chakra Listbox | SortableList imports Chakra Listbox and collection helpers |
| ✅ | Convert Tag component to Chakra Tag with deterministic palette | Tag uses ChakraTag and colorPalettes array |
| ✅ | Use Chakra CloseButton in relevant components | Task uses Chakra CloseButton instead of bootstrap |
| ✅ | Migrate Breadcrumbs/BreadcrumbItem to Chakra Breadcrumb | BreadcrumbItem and Breadcrumbs now use Chakra Breadcrumb elements |
| ✅ | Use Chakra Tabs in TabNavigation | TabNavigation imports Tabs from Chakra and removes Nav styling |
| ✅ | Introduce custom Dropdown (DropdownMenu) and replace bootstrap dropdowns | DropdownMenu component added and many components import it |
| ✅ | Add ChakraDataTable using TanStack Table with sorting/filtering/pagination and column meta | ChakraDataTable implementation added with TanStack imports |
| ✅ | Update Header, LanguageChooser, Class/Lesson/Session/User lists to use new Dropdown and Button | Header/LanguageChooser and multiple list components now import DropdownMenu/Button |
| ✅ | Update locales for datatable controls and task/reference-solution fields; simplify criteria labels | Locale JSONs modified with new keys and label text changes |
| ✅ | Track move/remove events for blocks in large stacks (scratch change) | should-track-move logic updated to use wasPartOfLargeStack |
| ✅ | Add dependencies: @chakra-ui/react, @tanstack/react-table, react-icons, next-themes | Dependencies added to frontend package.json |
| ✅ | Enable Next.js optimizePackageImports for Chakra | next.config.ts experimental.optimizePackageImports set |
| ✅ | Set tsconfig.moduleResolution to Bundler and target to esnext | tsconfig changes applied for moduleResolution and target |
| ✅ | Add ESLint rule to restrict @FortAwesome imports, encouraging react-icons | ESLint no-restricted-imports rule for @FortAwesome added |
| ✅ | Remove PrimeRange and react-bootstrap dropdown usage across the codebase | PrimeRange/Range files removed and bootstrap Dropdown imports removed in many files |
| ✅ | Wrap Storybook stories with ChakraProvider | Storybook preview adds ChakraProvider decorator |
| ✅ | Add TanStack Table type extensions and ColumnType enum | tanstack-types and tanstack.d.ts files added |
| ✅ | Add suppressHydrationWarning to _document for Chakra hydration guidance | _document.tsx modified to include suppressHydrationWarning on Html |
| ✅ | Adjust global styles (globals.scss) to align with new Chakra defaults/colors | globals.scss updated accent and button colors and removed universal selector |
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| testId={`lesson-${rowData.id}-actions-dropdown-button`} | ||
| > | ||
| <DropdownMenu.Item | ||
| onClick={() => { |
There was a problem hiding this comment.
Clicking the Edit button now bubbles up to the row and triggers onRowClick, so the detail redirect overrides the edit navigation. Stop propagation before pushing the edit route.
Prompt for AI agents
Address the following comment on frontend/src/components/lesson/LessonList.tsx at line 157:
<comment>Clicking the Edit button now bubbles up to the row and triggers `onRowClick`, so the detail redirect overrides the edit navigation. Stop propagation before pushing the edit route.</comment>
<file context>
@@ -133,33 +135,39 @@ const LessonList = ({
+ testId={`lesson-${rowData.id}-actions-dropdown-button`}
+ >
+ <DropdownMenu.Item
+ onClick={() => {
+ router.push(`/lesson/${rowData.id}/edit`);
+ }}
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
The propagation is stopped inside the dropdown component.
There was a problem hiding this comment.
Thanks for the feedback! I've updated an existing learning with this new information.
| trigger={ | ||
| <Button | ||
| variant={ButtonVariant.secondary} | ||
| onClick={() => { |
There was a problem hiding this comment.
Removing event suppression here causes the edit button to bubble up to the row’s onRowClick, so the edit navigation is immediately overridden by the detail navigation. Please keep the trigger from bubbling (e.g., by stopping propagation in the trigger or the DropdownMenu component) so the edit action remains usable.
Prompt for AI agents
Address the following comment on frontend/src/components/class/ClassList.tsx at line 131:
<comment>Removing event suppression here causes the edit button to bubble up to the row’s onRowClick, so the edit navigation is immediately overridden by the detail navigation. Please keep the trigger from bubbling (e.g., by stopping propagation in the trigger or the DropdownMenu component) so the edit action remains usable.</comment>
<file context>
@@ -124,12 +124,11 @@ const ClassList = () => {
variant={ButtonVariant.secondary}
- onClick={(e) => {
- e.stopPropagation();
+ onClick={() => {
router.push(`/class/${rowData.id}/edit`);
}}
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
The propagation is stopped inside the dropdown component.
There was a problem hiding this comment.
Thanks for the feedback! I already have a similar learning that covers this.
| } | ||
|
|
||
| return ( | ||
| <Fragment key={child.key}> |
There was a problem hiding this comment.
Using child.key directly here causes multiple fragments to share the key null whenever breadcrumb children were authored without explicit keys, producing React's missing-key warning. Provide a fallback (e.g., the Children.map index) when the child has no key.
Prompt for AI agents
Address the following comment on frontend/src/components/Breadcrumbs.tsx at line 25:
<comment>Using child.key directly here causes multiple fragments to share the key null whenever breadcrumb children were authored without explicit keys, producing React's missing-key warning. Provide a fallback (e.g., the Children.map index) when the child has no key.</comment>
<file context>
@@ -1,33 +1,35 @@
+ }
+
+ return (
+ <Fragment key={child.key}>
+ <Breadcrumb.Separator />
+ {child}
</file context>
| variant="secondary" | ||
| split | ||
| data-testid={`task-${rowData.id}-actions-dropdown-button`} | ||
| /> |
There was a problem hiding this comment.
The edit action button now renders with no icon or label, so the edit affordance disappears while the icon placed outside is non-interactive. Please move the icon back inside the button (or render an IconButton) so the edit action remains visible and clickable.
Prompt for AI agents
Address the following comment on frontend/src/components/task/TaskTable.tsx at line 109:
<comment>The edit action button now renders with no icon or label, so the edit affordance disappears while the icon placed outside is non-interactive. Please move the icon back inside the button (or render an IconButton) so the edit action remains visible and clickable.</comment>
<file context>
@@ -96,30 +98,37 @@ const TaskTable = () => {
}}
- data-testid={`task-${rowData.id}-delete-button`}
+ data-testid={`task-${rowData.id}-edit-button`}
+ />
+ <Icon>
+ <MdModeEdit />
</file context>
✅ Addressed in eeee72e
| columnType: ColumnType.icon, | ||
| }, | ||
| cell: ({ row }) => { | ||
| const status = row.original.firstTask; |
There was a problem hiding this comment.
The status column is reading firstTask instead of status, so the icon will not reflect the actual status value. Please change this to use the status field.
Prompt for AI agents
Address the following comment on frontend/src/components/ChakraDataTable.stories.tsx at line 77:
<comment>The status column is reading firstTask instead of status, so the icon will not reflect the actual status value. Please change this to use the status field.</comment>
<file context>
@@ -0,0 +1,191 @@
+ columnType: ColumnType.icon,
+ },
+ cell: ({ row }) => {
+ const status = row.original.firstTask;
+ if (status === "done") return <FaCheckCircle />;
+ if (status === "in progress") return <LuClock />;
</file context>
| const status = row.original.firstTask; | |
| const status = row.original.status; |
| trigger={ | ||
| <Button | ||
| variant={ButtonVariant.secondary} | ||
| onClick={() => { |
There was a problem hiding this comment.
Removing the stopPropagation on the edit trigger means the click now bubbles up to the table row’s onRowClick, so the user is sent to the session progress view instead of the edit screen. Please keep the event from bubbling (ideally inside DropdownMenu’s trigger) before removing it from this call site.
(Based on your team's feedback about centralizing click event suppression inside dropdown components.)
Prompt for AI agents
Address the following comment on frontend/src/components/session/SessionList.tsx at line 173:
<comment>Removing the `stopPropagation` on the edit trigger means the click now bubbles up to the table row’s `onRowClick`, so the user is sent to the session progress view instead of the edit screen. Please keep the event from bubbling (ideally inside `DropdownMenu`’s trigger) before removing it from this call site.
(Based on your team's feedback about centralizing click event suppression inside dropdown components.) </comment>
<file context>
@@ -166,12 +166,11 @@ const SessionList = ({ classId }: { classId: number }) => {
variant={ButtonVariant.secondary}
- onClick={(e) => {
- e.stopPropagation();
+ onClick={() => {
router.push(`/class/${classId}/session/${rowData.id}/edit`);
}}
</file context>
✅ Addressed in eeee72e
There was a problem hiding this comment.
The stop propagation is handled in the dropdown component
There was a problem hiding this comment.
Thanks for the feedback! I already have a similar learning that covers this.
| <Label>{intl.formatMessage(label)}</Label> | ||
| <InputWrapper> | ||
| <CurrentValue>{value}</CurrentValue> | ||
| <ChakraRange |
There was a problem hiding this comment.
Please forward the localized label to ChakraRange so the underlying slider exposes an accessible name (e.g., via Slider.Label); right now assistive technologies perceive the slider as unlabeled.
Prompt for AI agents
Address the following comment on frontend/src/components/form/ChakraRange.tsx at line 49:
<comment>Please forward the localized label to ChakraRange so the underlying slider exposes an accessible name (e.g., via `Slider.Label`); right now assistive technologies perceive the slider as unlabeled.</comment>
<file context>
@@ -1,11 +1,64 @@
+ <Label>{intl.formatMessage(label)}</Label>
+ <InputWrapper>
+ <CurrentValue>{value}</CurrentValue>
+ <ChakraRange
+ min={min}
+ max={max}
</file context>
There was a problem hiding this comment.
The bot has probably a point here.
| </Dropdown> | ||
| </div> | ||
| } | ||
| data-testid={`user-${rowData.id}-actions`} |
There was a problem hiding this comment.
DropdownMenu expects a testId prop, so this data-testid attribute is ignored and no test hook is emitted. Please pass testId instead so automated tests can find the element.
Prompt for AI agents
Address the following comment on frontend/src/components/user/UserList.tsx at line 97:
<comment>`DropdownMenu` expects a `testId` prop, so this `data-testid` attribute is ignored and no test hook is emitted. Please pass `testId` instead so automated tests can find the element.</comment>
<file context>
@@ -82,31 +82,31 @@ const UserList = () => {
<FontAwesomeIcon icon={faEdit} />
</Button>
}
+ data-testid={`user-${rowData.id}-actions`}
>
- <DropdownItem
</file context>
| data-testid={`user-${rowData.id}-actions`} | |
| testId={`user-${rowData.id}-actions`} |
There was a problem hiding this comment.
It's not wrong, it's literally your API @bubudetp :-)
const DropdownMenu = ({ trigger, children, testId }: DropdownProps) => {
| trigger={ | ||
| <Button | ||
| variant={ButtonVariant.secondary} | ||
| onClick={() => { |
There was a problem hiding this comment.
Dropping e.stopPropagation() here lets the table row click handler run, so clicking Edit redirects to the detail page instead of staying on the edit route. Please keep the propagation guard when invoking the router.
(Based on your team's feedback about centralizing click suppression inside Dropdown components—this trigger still needs the guard until DropdownMenu handles it.)
Prompt for AI agents
Address the following comment on frontend/src/components/user/UserList.tsx at line 89:
<comment>Dropping `e.stopPropagation()` here lets the table row click handler run, so clicking Edit redirects to the detail page instead of staying on the edit route. Please keep the propagation guard when invoking the router.
(Based on your team's feedback about centralizing click suppression inside Dropdown components—this trigger still needs the guard until DropdownMenu handles it.) </comment>
<file context>
@@ -82,31 +82,31 @@ const UserList = () => {
variant={ButtonVariant.secondary}
- onClick={(e) => {
- e.stopPropagation();
+ onClick={() => {
router.push(`/user/${rowData.id}/edit`);
}}
</file context>
| onClick={() => { | |
| onClick={(e) => { e.stopPropagation(); router.push(`/user/${rowData.id}/edit`); }} |
✅ Addressed in 3485b4c
There was a problem hiding this comment.
The propagation is handled in the dropdown component
There was a problem hiding this comment.
Thanks for the feedback! I've updated an existing learning with this new information.
bubudetp
force-pushed
the
feature/CRT-197_ChakraUIIntegration
branch
from
ba92758 to
68e35d3
Compare
frontend/src/components/Dropdown.tsx
Outdated
| const handleClick = (e: MouseEventReact<HTMLElement>) => { | ||
| if (onClick) { | ||
| onClick(e); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
I'm not sure why you need this. I believe passing onClick directly to Menu.Item would be strictly equivalent, unless the ClickHandler type has been defined incorrectly
frontend/src/components/Dropdown.tsx
Outdated
| import React, { useState, MouseEvent as MouseEventReact } from "react"; | ||
| import { LuChevronDown, LuChevronUp } from "react-icons/lu"; | ||
|
|
||
| type ClickHandler = (e: MouseEventReact<HTMLElement>) => void | Promise<void>; |
There was a problem hiding this comment.
If it's a Promise, it's not awaited. It might be best to only keep void in the API, to avoid giving the impression that we handle asynchronous callbacks correctly.
| icon={faEdit} | ||
| <Dropdown | ||
| trigger={ | ||
| <Button |
There was a problem hiding this comment.
I think this is still a relevant issue? 🤔
frontend/.eslintrc
Outdated
| } | ||
| ], | ||
| "@typescript-eslint/no-restricted-imports": ["error", { | ||
| "name": "@fortawesome", |
There was a problem hiding this comment.
I doubt this works, isn't our library @fontawesome ? :-)
| }) => ( | ||
| <StyledNav aria-label="breadcrumb"> | ||
| <StyledOl className="breadcrumb"> | ||
| <Breadcrumb.Root mt="4" p="2" fontWeight="bold"> |
There was a problem hiding this comment.
within reason, I would suggest using the full names of the properties, like padding instead of p. Also 4 and 2 here seem to be token values in the Chakra UI styling system. Where are the tokens defined?
There was a problem hiding this comment.
I'm very confused about this one.
Why do we need such a file and why can't we just rely on the tanstack typings?
There was a problem hiding this comment.
I'm a bit confused about why we need this 🤔
There was a problem hiding this comment.
We need this so that when we define columns for the Datatable we can pass custom metadata like this:
const columns: ColumnDef<YourData>[] = [ { accessorKey: "name", header: "Name", meta: { columnType: ColumnType.text, icon: <IconComponent />, }, }, { accessorKey: "link", header: "Link", meta: { columnType: ColumnType.link, linkHref: "/Login", }, }, ];
So in the component I can display them properly in the table cells. Let me know if this doesn't make sense
| <DropdownMenu | ||
| trigger={ | ||
| <Button | ||
| variant={ButtonVariant.secondary} |
There was a problem hiding this comment.
Ok, I think there was a misunderstanding 😅
When I mentioned the ButtonGroup being necessary here, I meant almost everywhere where the DropdownMenu has a trigger which is itself a Button with an onClick action, especially with navigation rules.
If you think about it, it means that "opening the drop-down" would automatically redirect me to a different page.
@cubic-dev-ai can you identify similar problems elsewhere to save us some time?
| <StyledInput {...inputProps} ref={ref} /> | ||
| {children} | ||
| <Field.Root invalid={invalid}> | ||
| {label && <Field.Label>{intl.formatMessage(label)}</Field.Label>} |
There was a problem hiding this comment.
@bubudetp I think this one still needs handling.
| {errors.title?.message} | ||
| </ValidationErrorMessage> | ||
| </Input> | ||
| errorText={errors.title?.message} |
There was a problem hiding this comment.
aren't we missing the invalid here and below?
bubudetp
force-pushed
the
feature/CRT-197_ChakraUIIntegration
branch
2 times, most recently
from
9da7c55 to
5f578f8
Compare
Tyratox
force-pushed
the
feature/CRT-197_ChakraUIIntegration
branch
from
5c71f35 to
8ab7af9
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| parseInt(option.value), | ||
| return this.inputs.teacherId.evaluate(() => | ||
| [...document.querySelectorAll('[role="option"]')].map((option) => | ||
| parseInt(option.getAttribute("data-value") || "0"), |
There was a problem hiding this comment.
Bug: Unscoped Query Returns All Page Options
The getTeacherIds() method uses document.querySelectorAll('[role="option"]') which queries the entire page instead of just the teacher select element. The callback previously received el (the select element) but now ignores it. This breaks when multiple option elements exist on the page, returning all options instead of just the teacher IDs, causing tests to parse incorrect values.
| <FormattedMessage | ||
| id="AnonymizationToggle.label" | ||
| defaultMessage="Anonymize Names" | ||
| /> |
There was a problem hiding this comment.
Bug: i18n Key Mismatch Breaks Localization
The component uses i18n message ID "AnonymizationToggle.label", but the locale files (en.json, fr.json) only contain "StudentAnonymizationToggle.showActualStudentNames". This message key mismatch means the component will always display the default message "Anonymize Names" instead of using translations, resulting in missing localization.
|
|
||
| await form.inputs.teacherId.selectOption(newClassTeacherId.toString()); | ||
| await form.inputs.teacherId.click(); | ||
| await pwPage.locator(`[data-value="${newClassTeacherId}"]`).click(); |
There was a problem hiding this comment.
Bug: E2E test assumes Chakra Select renders data-value attributes
The test attempts to select a Chakra Select option by clicking an element with selector [data-value="${newClassTeacherId}"], but Chakra UI's Select component doesn't document or guarantee rendering data-value attributes on option elements. The selector-based click will likely fail, causing teacher selection to fail in E2E tests.
| deploy: | ||
| needs: [tests] | ||
| # For now, we deploy even if tests fail. CHANGE THIS ASAP. | ||
| # needs: [tests] |
There was a problem hiding this comment.
Bug: Deployment workflow skips tests with temporary bypass comment
The deployment workflow has tests disabled with a comment "For now, we deploy even if tests fail. CHANGE THIS ASAP." This is a temporary debugging change that should not be in production. The needs: [tests] dependency should be re-enabled so deployments only proceed after tests pass. This is a critical safety issue for the deployment pipeline.
Requested by Laila
…18_show_modal_on_task_delete
…59_LimitedImportCapabilities
…/CRT-163_LimitedExportCapabilities
…m:crt25/collimator into feature/CRT-163_LimitedExportCapabilities
…ilities [CRT-163] Limited export capabilities
…tipleCopies [CRT-233] Block drag creates multiple copies
…59_LimitedImportCapabilities
…8_remove_old_solutions_from_showcase
…ilities [CRT-159] Limited import capabilities
…s_from_showcase [CRT-238] Add ability to remove old solutions from showcase
|
To be reviewed:
Summary by cubic
Migrates the frontend to Chakra UI and replaces legacy tables with a TanStack-powered DataTable for faster, cleaner lists. Adds task import/export across Jupyter and Scratch, refreshes navigation/dashboards, and updates i18n/config; addresses CRT-197.
New Features
Bug Fixes
Written for commit 4c03348. Summary will update automatically on new commits.
Note
Replaces Bootstrap/Prime UI with Chakra UI + themed components, introduces a reusable TanStack-powered data table, and overhauls class/lesson/task pages (incl. task instances, showcase, analysis, progress) alongside dashboard and hooks refactors.
@chakra-ui/reactwith custom theme tokens/recipes and provider; remove legacy Bootstrap/Prime usage in favor of Chakra components (buttons, inputs, selects, dropdowns, modals, avatar menu, breadcrumbs, tabs, tags, pagination, toaster, footer, anonymization toggle, page headings).ChakraDataTable(TanStack React Table) with sorting/filtering/search/pagination; apply to class, lesson (session), task, student, and task-instance lists.LazyTableAPIs; simplify SWR keys; add solution revalidation utilities; harden solutions/task-instance hooks and helpers.optimizePackageImportsfor Chakra; addchakra:updatescript; update tsconfig (Bundler, esnext); addreact-icons,@tanstack/react-table,next-themes; dropreact-bootstrap/legacy range/react-hot-toast; update styles and i18n content.Written by Cursor Bugbot for commit 465c716. This will update automatically on new commits. Configure here.