feat(ux): inline editor + visual grouper + scroll fixes for meus treinos#175
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
- Add compact?: boolean prop (default false) to WorkoutEditor - Extract JSX into const content, conditionally wrap in div vs Card - Add 2 tests: compact omits Card, default renders Card Co-Authored-By: Claude <noreply@anthropic.com>
- editingTreinoId state drives inline WorkoutEditor per card - handleEditLocal replaces scrollTo behavior - Criar Manual button moved above list, opens inline editor - Removed bottom-of-page conditional editor - Removed window.scrollTo from use-workout-crud Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Ensures banner re-triggers on every AI gen, not just first time. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Replaces fixed 300ms setTimeout with rAF loop that retries until #treinos-pessoais anchor exists in DOM. Eliminates race condition on slow devices / slow RSC renders. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
WalkthroughThis PR adds a ChangesMeus Treinos UX and Backend Fixes
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Student
participant MeusTreinosClient
participant useWorkoutGeneration
participant Router
Student->>MeusTreinosClient: click Gerar Treino
MeusTreinosClient->>useWorkoutGeneration: handleGenerate()
useWorkoutGeneration-->>MeusTreinosClient: onSuccess(planName)
MeusTreinosClient->>Router: router.refresh()
MeusTreinosClient->>MeusTreinosClient: show Plano Semanal banner
MeusTreinosClient->>MeusTreinosClient: scroll to treinos-pessoais (rAF retry)
MeusTreinosClient->>MeusTreinosClient: auto-hide banner after 30s
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 9 files
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="src/app/aluno/meus-treinos/meus-treinos-client.tsx">
<violation number="1" location="src/app/aluno/meus-treinos/meus-treinos-client.tsx:138">
P2: When a save request fails, the inline editor is still closed immediately because `setEditingTreinoId(null)` runs right after calling `handleSave(...)` without waiting for success. This regresses the previous behavior where the form stayed open on error, so users can lose what they just typed. A safer flow is to close the inline editor only after a confirmed successful save result.</violation>
<violation number="2" location="src/app/aluno/meus-treinos/meus-treinos-client.tsx:277">
P2: Custom agent: **Enforce Strict Maintainability Standards**
The `__new__` magic-string sentinel overloads `editingTreinoId` to mean both "editing an existing treino" (when it holds a real ID) and "creating a new treino" (when it equals `__new__`). A `string | null` type can only express two states, so the third state is hidden by an implicit convention that every reader must discover. Replacing this with a discriminated union or a dedicated boolean flag would make the state machine explicit and self-documenting, and remove the risk of the sentinel drifting out of sync if it is ever changed in one place but not the other.</violation>
</file>
<file name="src/hooks/use-workout-generation.ts">
<violation number="1" location="src/hooks/use-workout-generation.ts:81">
P2: `planName` state is set inside the success block but never cleaned up when re-invocation fails mid-way. If a second generation is started while the first is still in-flight, `setPlanName(null)` runs immediately but the in-flight success could set `planName` to stale data after the second generation completes. Consider using an abort/mount ref or resetting `planName` only at the start of a new generation (already done) and protecting `setPlanName` from stale completions.</violation>
</file>
<file name="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md">
<violation number="1" location="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md:9">
P3: The implementation plan now documents `React 18`, but this repository is on React 19.x. That mismatch can lead to incorrect implementation/testing guidance (especially around runtime/test behavior), so updating the plan to the actual version would keep the UX task instructions reliable.</violation>
<violation number="2" location="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md:337">
P1: Test mock setup is broken: Step 1 replaces `useWorkoutGeneration: vi.fn(() => ({...}))` with `useWorkoutGeneration: () => ({...})`, dropping the `vi.fn()` wrapper. Then Step 3 attempts `(useWorkoutGeneration as ReturnType<typeof vi.fn>).mockReturnValue(...)` — but since `useWorkoutGeneration` is now a plain arrow function, `.mockReturnValue` is undefined and the test will throw a TypeError at runtime. Keep `vi.fn(() => ...)` in the mock factory so the dynamic override in the banner test works. The `as ReturnType<typeof vi.fn>` cast only silences TypeScript, it doesn't create the mock method at runtime.</violation>
</file>
<file name="src/hooks/use-workout-crud.ts">
<violation number="1" location="src/hooks/use-workout-crud.ts:28">
P2: `editingTreino` and `isFormVisible` state inside `useWorkoutCRUD` is now maintained but unused after the refactor. The consumer (`meus-treinos-client.tsx`) manages its own `editingTreinoId` state and doesn't destructure these from the hook. `handleEdit` still sets them and `handleSave` still resets them, but nothing reads them — creates unnecessary state management overhead and could mislead future maintainers into thinking the hook controls form visibility. Suggest removing `editingTreino`, `isFormVisible`, and their setters from the hook since the consumer owns inline editing state externally.</violation>
</file>
<file name="docs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.md">
<violation number="1" location="docs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.md:69">
P2: The design doc now points implementation to `card-treino.tsx`, while the implementation plan explicitly says inline editing must not be done there. This conflict can send contributors to modify the wrong component path, so aligning both docs to the same target file(s) would avoid rework.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| ```typescript | ||
| // Lines 57-62, add planName to hook mock return: | ||
| vi.mock('@/hooks/use-workout-generation', () => ({ |
There was a problem hiding this comment.
P1: Test mock setup is broken: Step 1 replaces useWorkoutGeneration: vi.fn(() => ({...})) with useWorkoutGeneration: () => ({...}), dropping the vi.fn() wrapper. Then Step 3 attempts (useWorkoutGeneration as ReturnType<typeof vi.fn>).mockReturnValue(...) — but since useWorkoutGeneration is now a plain arrow function, .mockReturnValue is undefined and the test will throw a TypeError at runtime. Keep vi.fn(() => ...) in the mock factory so the dynamic override in the banner test works. The as ReturnType<typeof vi.fn> cast only silences TypeScript, it doesn't create the mock method at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md, line 337:
<comment>Test mock setup is broken: Step 1 replaces `useWorkoutGeneration: vi.fn(() => ({...}))` with `useWorkoutGeneration: () => ({...})`, dropping the `vi.fn()` wrapper. Then Step 3 attempts `(useWorkoutGeneration as ReturnType<typeof vi.fn>).mockReturnValue(...)` — but since `useWorkoutGeneration` is now a plain arrow function, `.mockReturnValue` is undefined and the test will throw a TypeError at runtime. Keep `vi.fn(() => ...)` in the mock factory so the dynamic override in the banner test works. The `as ReturnType<typeof vi.fn>` cast only silences TypeScript, it doesn't create the mock method at runtime.</comment>
<file context>
@@ -0,0 +1,431 @@
+
+```typescript
+// Lines 57-62, add planName to hook mock return:
+vi.mock('@/hooks/use-workout-generation', () => ({
+ useWorkoutGeneration: () => ({
+ isGenerating: false,
</file context>
| <div className="text-center mt-4"> | ||
| <Button | ||
| onClick={() => { | ||
| setEditingTreinoId('__new__'); |
There was a problem hiding this comment.
P2: Custom agent: Enforce Strict Maintainability Standards
The __new__ magic-string sentinel overloads editingTreinoId to mean both "editing an existing treino" (when it holds a real ID) and "creating a new treino" (when it equals __new__). A string | null type can only express two states, so the third state is hidden by an implicit convention that every reader must discover. Replacing this with a discriminated union or a dedicated boolean flag would make the state machine explicit and self-documenting, and remove the risk of the sentinel drifting out of sync if it is ever changed in one place but not the other.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/app/aluno/meus-treinos/meus-treinos-client.tsx, line 277:
<comment>The `__new__` magic-string sentinel overloads `editingTreinoId` to mean both "editing an existing treino" (when it holds a real ID) and "creating a new treino" (when it equals `__new__`). A `string | null` type can only express two states, so the third state is hidden by an implicit convention that every reader must discover. Replacing this with a discriminated union or a dedicated boolean flag would make the state machine explicit and self-documenting, and remove the risk of the sentinel drifting out of sync if it is ever changed in one place but not the other.</comment>
<file context>
@@ -201,44 +246,57 @@ export default function MeusTreinosClient({
+ <div className="text-center mt-4">
+ <Button
+ onClick={() => {
+ setEditingTreinoId('__new__');
+ }}
+ variant="outline"
</file context>
| } | ||
| } | ||
|
|
||
| setPlanName(result.planName); |
There was a problem hiding this comment.
P2: planName state is set inside the success block but never cleaned up when re-invocation fails mid-way. If a second generation is started while the first is still in-flight, setPlanName(null) runs immediately but the in-flight success could set planName to stale data after the second generation completes. Consider using an abort/mount ref or resetting planName only at the start of a new generation (already done) and protecting setPlanName from stale completions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/use-workout-generation.ts, line 81:
<comment>`planName` state is set inside the success block but never cleaned up when re-invocation fails mid-way. If a second generation is started while the first is still in-flight, `setPlanName(null)` runs immediately but the in-flight success could set `planName` to stale data after the second generation completes. Consider using an abort/mount ref or resetting `planName` only at the start of a new generation (already done) and protecting `setPlanName` from stale completions.</comment>
<file context>
@@ -76,6 +78,7 @@ export function useWorkoutGeneration({
}
}
+ setPlanName(result.planName);
notify.success('Plano Pessoal Gerado!', `${result.planName} foi criado com sucesso.`);
onSuccess();
</file context>
|
|
||
| | File | Change | Risk | | ||
| |---|---|---| | ||
| | `card-treino.tsx` | +isEditing state, inline WorkoutEditor render | Low | |
There was a problem hiding this comment.
P2: The design doc now points implementation to card-treino.tsx, while the implementation plan explicitly says inline editing must not be done there. This conflict can send contributors to modify the wrong component path, so aligning both docs to the same target file(s) would avoid rework.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.md, line 69:
<comment>The design doc now points implementation to `card-treino.tsx`, while the implementation plan explicitly says inline editing must not be done there. This conflict can send contributors to modify the wrong component path, so aligning both docs to the same target file(s) would avoid rework.</comment>
<file context>
@@ -0,0 +1,88 @@
+
+| File | Change | Risk |
+|---|---|---|
+| `card-treino.tsx` | +isEditing state, inline WorkoutEditor render | Low |
+| `workout-editor.tsx` | +`compact?: boolean` prop | Low |
+| `meus-treinos-client.tsx` | -bottom editor, +banner, +planName, +scroll anchor | Medium |
</file context>
|
|
||
| **Architecture:** State `editingTreinoId` drives which card shows inline `WorkoutEditor`. `WorkoutEditor` gets `compact` prop to drop outer Card wrapper. `useWorkoutGeneration` exposes `planName`. Banner + scroll driven by local state in `meus-treinos-client.tsx`. | ||
|
|
||
| **Tech Stack:** React 18, TypeScript 5, Next.js 15 App Router, Vitest |
There was a problem hiding this comment.
P3: The implementation plan now documents React 18, but this repository is on React 19.x. That mismatch can lead to incorrect implementation/testing guidance (especially around runtime/test behavior), so updating the plan to the actual version would keep the UX task instructions reliable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md, line 9:
<comment>The implementation plan now documents `React 18`, but this repository is on React 19.x. That mismatch can lead to incorrect implementation/testing guidance (especially around runtime/test behavior), so updating the plan to the actual version would keep the UX task instructions reliable.</comment>
<file context>
@@ -0,0 +1,431 @@
+
+**Architecture:** State `editingTreinoId` drives which card shows inline `WorkoutEditor`. `WorkoutEditor` gets `compact` prop to drop outer Card wrapper. `useWorkoutGeneration` exposes `planName`. Banner + scroll driven by local state in `meus-treinos-client.tsx`.
+
+**Tech Stack:** React 18, TypeScript 5, Next.js 15 App Router, Vitest
+
+## Global Constraints
</file context>
…rization deleteTreinoAction and updateTreinoDayAction compared treino.alunoId (Prisma ID) against user.id (Auth UUID), always mismatching after PR #174 FK fix. Resolve aluno.id by email before auth check. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="src/hooks/use-workout-generation.ts">
<violation number="1" location="src/hooks/use-workout-generation.ts:81">
P2: `planName` state is set inside the success block but never cleaned up when re-invocation fails mid-way. If a second generation is started while the first is still in-flight, `setPlanName(null)` runs immediately but the in-flight success could set `planName` to stale data after the second generation completes. Consider using an abort/mount ref or resetting `planName` only at the start of a new generation (already done) and protecting `setPlanName` from stale completions.</violation>
</file>
<file name="src/app/aluno/meus-treinos/meus-treinos-client.tsx">
<violation number="1" location="src/app/aluno/meus-treinos/meus-treinos-client.tsx:277">
P2: Custom agent: **Enforce Strict Maintainability Standards**
The `__new__` magic-string sentinel overloads `editingTreinoId` to mean both "editing an existing treino" (when it holds a real ID) and "creating a new treino" (when it equals `__new__`). A `string | null` type can only express two states, so the third state is hidden by an implicit convention that every reader must discover. Replacing this with a discriminated union or a dedicated boolean flag would make the state machine explicit and self-documenting, and remove the risk of the sentinel drifting out of sync if it is ever changed in one place but not the other.</violation>
</file>
<file name="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md">
<violation number="1" location="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md:9">
P3: The implementation plan now documents `React 18`, but this repository is on React 19.x. That mismatch can lead to incorrect implementation/testing guidance (especially around runtime/test behavior), so updating the plan to the actual version would keep the UX task instructions reliable.</violation>
<violation number="2" location="docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.md:337">
P1: Test mock setup is broken: Step 1 replaces `useWorkoutGeneration: vi.fn(() => ({...}))` with `useWorkoutGeneration: () => ({...})`, dropping the `vi.fn()` wrapper. Then Step 3 attempts `(useWorkoutGeneration as ReturnType<typeof vi.fn>).mockReturnValue(...)` — but since `useWorkoutGeneration` is now a plain arrow function, `.mockReturnValue` is undefined and the test will throw a TypeError at runtime. Keep `vi.fn(() => ...)` in the mock factory so the dynamic override in the banner test works. The `as ReturnType<typeof vi.fn>` cast only silences TypeScript, it doesn't create the mock method at runtime.</violation>
</file>
<file name="docs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.md">
<violation number="1" location="docs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.md:69">
P2: The design doc now points implementation to `card-treino.tsx`, while the implementation plan explicitly says inline editing must not be done there. This conflict can send contributors to modify the wrong component path, so aligning both docs to the same target file(s) would avoid rework.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- rAF scroll capped at 20 attempts (prevents infinite loop) - handleDelete + handleDayChange: surface server errors to user - replace inline prisma.aluno.findUnique with resolveAlunoId() for auth - add onSuccess to useCallback dep array Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Pre-existing tests don't pass onSuccess — made optional to avoid 12 TS2345 errors. Caller (meus-treinos-client) always passes it. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/use-workout-crud.ts (1)
20-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate edit-state ownership in
useWorkoutCRUDOnlyMeusTreinosClientconsumes this hook now, and it keepseditingTreinoIdlocally. The hook’sisFormVisible/editingTreino/handleEditstate is redundant and can be removed or folded into a single source of truth.🤖 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 `@src/hooks/use-workout-crud.ts` around lines 20 - 25, In useWorkoutCRUD, remove the redundant edit-form state owned by the hook: isFormVisible, editingTreino, and handleEdit should no longer be separate sources of truth since MeusTreinosClient now tracks editingTreinoId locally. Update useWorkoutCRUD to keep only the CRUD state it still owns, and adjust any returned values or handlers so edit behavior is driven from the client component’s single edit-state flow.
🤖 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 `@src/app/aluno/meus-treinos/meus-treinos-client.tsx`:
- Around line 82-92: The plan banner can remain visible forever if a new
generation starts and then fails because the existing timer is cleared by the
`useEffect` cleanup when `planName` becomes null. Update
`meus-treinos-client.tsx` so the banner is explicitly hidden at the start of the
generation flow (for example in `onGenerate` or right before `handleGenerate`
runs) by resetting both `setShowPlanBanner(false)` and
`setBannerPlanName(null)`, instead of relying only on the `planName` effect in
`useEffect`.
---
Nitpick comments:
In `@src/hooks/use-workout-crud.ts`:
- Around line 20-25: In useWorkoutCRUD, remove the redundant edit-form state
owned by the hook: isFormVisible, editingTreino, and handleEdit should no longer
be separate sources of truth since MeusTreinosClient now tracks editingTreinoId
locally. Update useWorkoutCRUD to keep only the CRUD state it still owns, and
adjust any returned values or handlers so edit behavior is driven from the
client component’s single edit-state flow.
🪄 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: CHILL
Plan: Pro
Run ID: a09edfef-b533-464c-9bd0-cf5979a7e387
📒 Files selected for processing (10)
docs/superpowers/plans/2026-07-05-meus-treinos-ux-improvements.mddocs/superpowers/specs/2026-07-05-meus-treinos-ux-improvements-design.mdsrc/app/aluno/meus-treinos/meus-treinos-client.test.tsxsrc/app/aluno/meus-treinos/meus-treinos-client.tsxsrc/components/dashboard/aluno/workout-editor.test.tsxsrc/components/dashboard/aluno/workout-editor.tsxsrc/hooks/use-workout-crud.test.tssrc/hooks/use-workout-crud.tssrc/hooks/use-workout-generation.tssrc/lib/actions/treinos.ts
Comment on lines
+82
to
+92
| const [showPlanBanner, setShowPlanBanner] = useState(false); | ||
| const [bannerPlanName, setBannerPlanName] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| if (planName) { | ||
| setBannerPlanName(planName); | ||
| setShowPlanBanner(true); | ||
| const timer = setTimeout(() => setShowPlanBanner(false), 30000); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [planName]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Banner can get stuck if a subsequent generation attempt fails.
Starting a new handleGenerate call resets planName to null, which triggers this effect's cleanup and cancels the pending 30s auto-hide timer for the currently visible banner — but nothing re-arms it if the new generation then fails. The banner (with the stale plan name) will stay visible indefinitely with no way to auto-clear.
🐛 Suggested fix: explicitly hide the banner when a new generation starts
+ const [isRegenerating, setIsRegenerating] = useState(false);
+
useEffect(() => {
if (planName) {
setBannerPlanName(planName);
setShowPlanBanner(true);
const timer = setTimeout(() => setShowPlanBanner(false), 30000);
return () => clearTimeout(timer);
+ } else if (isGenerating) {
+ setShowPlanBanner(false);
}
- }, [planName]);
+ }, [planName, isGenerating]);As an alternative, simply hide the banner explicitly at the start of onGenerate/before calling handleGenerate, rather than relying solely on the planName reset to null.
🤖 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 `@src/app/aluno/meus-treinos/meus-treinos-client.tsx` around lines 82 - 92, The
plan banner can remain visible forever if a new generation starts and then fails
because the existing timer is cleared by the `useEffect` cleanup when `planName`
becomes null. Update `meus-treinos-client.tsx` so the banner is explicitly
hidden at the start of the generation flow (for example in `onGenerate` or right
before `handleGenerate` runs) by resetting both `setShowPlanBanner(false)` and
`setBannerPlanName(null)`, instead of relying only on the `planName` effect in
`useEffect`.
handleSave returns boolean. Client closes editor only on success. Removed unused isFormVisible, editingTreino, handleEdit from hook. Kept __new__ sentinel for now. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="src/app/aluno/meus-treinos/meus-treinos-client.tsx">
<violation number="1" location="src/app/aluno/meus-treinos/meus-treinos-client.tsx:142">
P2: The async `onSave` callback unconditionally clears the global `editingTreinoId` after a successful save. Because other treino cards remain interactive during the save request, a user could switch to editing a different treino before the first save resolves. When the late success fires, it will close the wrong inline editor and potentially discard in-progress edits.
A safer pattern is to clear the editor only if the same treino is still being edited by using a functional state update.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| treinoToEdit={treino} | ||
| onSave={async (data) => { | ||
| const ok = await handleSave(data, treino.id); | ||
| if (ok) setEditingTreinoId(null); |
There was a problem hiding this comment.
P2: The async onSave callback unconditionally clears the global editingTreinoId after a successful save. Because other treino cards remain interactive during the save request, a user could switch to editing a different treino before the first save resolves. When the late success fires, it will close the wrong inline editor and potentially discard in-progress edits.
A safer pattern is to clear the editor only if the same treino is still being edited by using a functional state update.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/app/aluno/meus-treinos/meus-treinos-client.tsx, line 142:
<comment>The async `onSave` callback unconditionally clears the global `editingTreinoId` after a successful save. Because other treino cards remain interactive during the save request, a user could switch to editing a different treino before the first save resolves. When the late success fires, it will close the wrong inline editor and potentially discard in-progress edits.
A safer pattern is to clear the editor only if the same treino is still being edited by using a functional state update.</comment>
<file context>
@@ -137,9 +137,9 @@ export default function MeusTreinosClient({
- setEditingTreinoId(null);
+ onSave={async (data) => {
+ const ok = await handleSave(data, treino.id);
+ if (ok) setEditingTreinoId(null);
}}
onCancel={() => setEditingTreinoId(null)}
</file context>
Suggested change
| if (ok) setEditingTreinoId(null); | |
| if (ok) setEditingTreinoId((current) => (current === treino.id ? null : current)); |
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
1 participant
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.
Changes
Files (6)
Review
Closes UX issues from design spec 2026-07-05.
🤖 Generated with Claude Code
Summary by cubic
Inline editor inside treino cards, a short “Plano Semanal” banner after generation, and smooth scroll back to the treino list. The create button now sits above the list; the bottom editor is removed, with sturdier auth and error handling.
New Features
WorkoutEditoraddscompactand renders inline in treino cards; create opens inline above the list.use-workout-generationexposes and resetsplanName.Bug Fixes
#treinos-pessoaisafter generation with rAF retry capped at 20; removedwindow.scrollTo.updateTreinoDayActionanddeleteTreinoActionby resolvingaluno.id;handleDayChange/handleDeletenow surface server errors.onSuccessoptional inuse-workout-generationto resolve typing issues in tests.Written for commit 2133cfe. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes