fix(uikit-playground): update activeScreen/activeProject correctly after deletion#40020
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (3)📚 Learning: 2026-03-04T14:16:49.202ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
🔇 Additional comments (2)
WalkthroughAdjusts reducer deletion logic in the UIKit Playground: deleting a screen now removes it from its project and related flow edges/nodes before recalculating Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
1 issue found across 1 file
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="apps/uikit-playground/src/Context/reducer.ts">
<violation number="1" location="apps/uikit-playground/src/Context/reducer.ts:302">
P2: Deleting a non-active project incorrectly forces `activeProject` to the first remaining project, overriding the current active selection.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/uikit-playground/src/Context/reducer.ts`:
- Around line 197-206: The reducer always sets state.activeScreen =
project.screens[0] after deletion which changes selection incorrectly; instead,
compute the new screens array (project.screens filtered) and if the deleted id
(action.payload) was not the current state.activeScreen leave state.activeScreen
untouched, but if it was the active one choose the screen at the same index in
the filtered array (i.e., keep the slot: use the index of the deleted item in
the original project.screens to pick the element at that index in the filtered
project.screens if present, otherwise pick the previous element) — update the
logic around state.projects[activeProject], project.screens, and
state.activeScreen to implement this selection-preserving behavior.
- Around line 297-303: The reducer currently resets activeProject
unconditionally after any deletion; change it so the reset only happens when the
deleted project is the currently active one by checking if action.payload ===
state.activeProject before assigning activeProject and activeScreen; update the
block around activeProject/activeScreen (in the reducer handling project
deletions) to gate the assignment with that conditional and keep the existing
state otherwise (and still handle the case where there are no remaining
projects).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ada45a2-eac8-44b6-84b3-4a667cfb7778
📒 Files selected for processing (1)
apps/uikit-playground/src/Context/reducer.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/uikit-playground/src/Context/reducer.ts
🧠 Learnings (2)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/uikit-playground/src/Context/reducer.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/uikit-playground/src/Context/reducer.ts
…agement - DeleteScreen: only update activeScreen when the deleted screen was the active one, and select the nearest screen by index instead of always picking the first - DeleteScreen/DeleteProject: create a default project+screen instead of setting activeProject/activeScreen to empty strings, preventing crashes in useNodesAndEdges, PrototypeContainer, and SurfaceSelect - Remove unnecessary `|| ''` fallbacks now that empty state is handled Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Description
Fixes incorrect state management in UIKit Playground when deleting screens and projects.
Related Issue
Closes #39240
Changes Made
DeleteScreen reducer:
state.projects[activeProject].flowEdges— causing a crashbecause the project no longer existed
activeProjectandactiveScreennow fall back to the first remaining project instead of leaving stalereferences
DeleteProject reducer:
activeProject: ''even when other projects existed. Now falls back to the first remaining project.map()with.forEach()for the delete side effect (lint best practice)console.logfrom production codeManual Testing
Summary by CodeRabbit
Bug Fixes
Chores