Repository navigation
feat(ui): refresh onboarding with refined ProductOS UX - #158
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a UX flow audit and change map; standardizes UI copy to "Product" and "App settings"; introduces ProductHome and MainPanel responsive/stacked layout; refactors Welcome; adds chat hint chips and workflow delete confirmation; updates E2E tests and skill-import API payload. ChangesProductOS Terminology & Welcome Page Redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/ux-flow-map.md`:
- Line 151: The markdown table cell showing the logical operator in the docs can
be mis-parsed; update the cell that references handleProjectSelect and the
expression involving firstChat and first document by replacing the literal "||"
with an escaped form (e.g. use the HTML entity for pipe) so the operator renders
correctly inside the table cell—modify the table row that mentions
handleProjectSelect / firstChat || first document to use the escaped pipe
sequence instead and keep the surrounding explanatory text intact.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7d951bd-8684-4677-9f7d-e0bc143f7dde
⛔ Files ignored due to path filters (4)
docs/mockups/welcome-accomplish-inspired.pngis excluded by!**/*.pngdocs/mockups/welcome-accomplish-inspired.svgis excluded by!**/*.svgdocs/mockups/workspace-product-settings.pngis excluded by!**/*.pngdocs/mockups/workspace-product-settings.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
docs/README.mddocs/ux-flow-map.mdsrc/components/workspace/MainPanel.tsxsrc/components/workspace/ProjectFormDialog.tsxsrc/components/workspace/Sidebar.tsxsrc/components/workspace/TopBar.tsxsrc/pages/Welcome.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/ux-change-map.md (1)
57-59: ⚡ Quick winNormalize file-path notation in “Main surfaces”
This section mixes repo-relative paths and filename-only references (e.g., Line 57 vs Line 58/59). Standardizing on repo-relative paths will make implementation handoff less ambiguous.
🤖 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 `@docs/ux-change-map.md` around lines 57 - 59, The "Main surfaces" table mixes repo-relative paths and filename-only references; update all filename-only entries to repo-relative paths for consistency. Edit the table rows referencing Welcome.tsx and ProviderSettings.tsx so they read src/pages/Welcome.tsx and src/pages/ProviderSettings.tsx, and ensure other entries (e.g., Onboarding.tsx, App.tsx) use the repo-relative form (src/pages/Onboarding.tsx, src/App.tsx) so every cell follows the same convention.
🤖 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 `@docs/ux-change-map.md`:
- Line 123: Update the table row containing "P2 | Make rail tooltips always
clear | Every rail icon should have a precise tooltip and active state. |
`Sidebar.tsx` | Low risk polish." to hyphenate the compound adjective by
replacing "Low risk polish." with "Low-risk polish."; locate the entry by
searching for the exact row text or the `Sidebar.tsx` reference and modify the
final cell accordingly.
- Line 26: The P0 legend in the table is a fragment; rewrite the P0 row so it is
a self-contained full sentence that clearly states the priority and required
action (e.g., "P0 — Safety, trust, and scope clarity: must be resolved before
broad release."). Update the P0 table cell text (the "P0" legend entry) to this
full-sentence form to improve scanability and consistency with other legend
rows.
---
Nitpick comments:
In `@docs/ux-change-map.md`:
- Around line 57-59: The "Main surfaces" table mixes repo-relative paths and
filename-only references; update all filename-only entries to repo-relative
paths for consistency. Edit the table rows referencing Welcome.tsx and
ProviderSettings.tsx so they read src/pages/Welcome.tsx and
src/pages/ProviderSettings.tsx, and ensure other entries (e.g., Onboarding.tsx,
App.tsx) use the repo-relative form (src/pages/Onboarding.tsx, src/App.tsx) so
every cell follows the same convention.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca351611-4629-4fea-adee-0ce1d9ad1dd7
📒 Files selected for processing (2)
docs/README.mddocs/ux-change-map.md
✅ Files skipped from review due to trivial changes (1)
- docs/README.md
aviatam
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
docs/ux-change-map.md (2)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGrammar: Make the P0 legend a complete sentence.
The P0 row in the priority legend table reads as a fragment. Consider rephrasing to: "P0 — Safety, trust, and scope clarity that should be addressed before broad release."
🤖 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 `@docs/ux-change-map.md` at line 25, Update the P0 legend text in the priority table (the row starting with "P0") so it becomes a complete sentence; replace the fragment "Safety / trust / scope clarity. Should be addressed before broad release." with "P0 — Safety, trust, and scope clarity that should be addressed before broad release." to fix the grammar.
122-122:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGrammar: Hyphenate the compound adjective.
The phrase "Low risk polish" should use a hyphen: "Low-risk polish."
🤖 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 `@docs/ux-change-map.md` at line 122, The table row describing the priority entry uses "Low risk polish" and needs the compound adjective hyphenated; update the text to "Low-risk polish" in the markdown table entry (the row referencing `Sidebar.tsx`) so the phrasing is grammatically correct.
🧹 Nitpick comments (3)
e2e/layout.spec.ts (1)
83-84: ⚖️ Poor tradeoffConsider: Hardcoded color check may be brittle.
The assertion
expect(headingColor).toBe('rgb(255, 255, 255)')verifies the heading is white, which is correct for the current dark theme. However, this check will break if theme colors are adjusted in the future. Consider whether this is essential to verify or could be replaced with a contrast-ratio check.src/components/workspace/MainPanel.tsx (2)
23-23: ⚡ Quick winUse the frontend alias for the new
ProductHomeimport.This new relative import breaks the repo’s frontend import convention and makes later moves/refactors harder.
Suggested change
-import ProductHome from '../../pages/ProductHome'; +import ProductHome from '@/pages/ProductHome';As per coding guidelines,
Use@/path alias for all internal module imports in React/TypeScript frontend code (maps to ./src/).🤖 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/components/workspace/MainPanel.tsx` at line 23, Replace the relative import of ProductHome in MainPanel.tsx with the frontend path alias; update the import statement that currently references '../../pages/ProductHome' to use '@/pages/ProductHome' so it follows the repo convention for internal module imports and remains resilient to future moves/refactors (locate the import line for ProductHome in the MainPanel component and change the path accordingly).
279-281: ⚡ Quick winPrefer semantic color tokens in the new empty-state styling.
The new callout adds hard-coded
white/emeraldutilities, which will drift from the app theme faster than the existing semantic tokens.As per coding guidelines,
Use Tailwind CSS with custom HSL color variables for styling in React components.🤖 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/components/workspace/MainPanel.tsx` around lines 279 - 281, The empty-state span in MainPanel.tsx uses hard-coded white/emerald utilities (the span with className containing border-white/10, bg-white/[0.045], and the Sparkles using text-emerald-300); replace these with the app’s semantic Tailwind token classes (use the HSL-based custom variables for border, background and icon color tokens used elsewhere, e.g., border-[token], bg-[token], text-[token]) so the callout follows theme tokens instead of raw colors while keeping the same layout and the Sparkles icon.
🤖 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/components/workspace/MainPanel.tsx`:
- Around line 278-282: The empty-state callout guarded by openDocuments.length
=== 0 never appears because the tabs/editor shell is only mounted when
hasContentArea is true; update MainPanel to render the callout when the content
area is not mounted (e.g., when !hasContentArea) or move the span out of the
hasContentArea-dependent shell so it is rendered whenever openDocuments.length
=== 0 regardless of hasContentArea; locate the check using the openDocuments
array and the hasContentArea flag in the MainPanel component and adjust the
conditional logic to ensure the callout is displayed in the empty state.
In `@src/pages/Workspace.tsx`:
- Around line 70-75: getProductHomeDocument currently bakes
WorkspaceProject.name into the Document.name causing stale tab labels after a
rename; change the flow so the Document id remains `product-home-${project.id}`
but its name is not permanently baked (e.g., leave name empty or use a neutral
label) and ensure the UI computes the visible tab label from the current
WorkspaceProject map at render time (or update open documents on rename).
Specifically, update getProductHomeDocument to stop embedding project.name and
modify the tab rendering logic (or the project rename handler) to derive or
refresh the label for any open document whose id matches
`product-home-${project.id}` using the latest WorkspaceProject.name so tabs
always show the current product name.
- Around line 378-383: The current branch that opens the product home only runs
when activeDocument is falsy or is a welcome/product-settings document, so if a
file from product A is open and the user selects product B the code leaves
activeDocument pointing at A and MainPanel shows “File Unavailable.” Update the
conditional around getProductHomeDocument/setOpenDocuments/setActiveDocument to
also trigger when activeDocument exists but belongs to a different product (e.g.
compare activeDocument.productId (or activeDocument.projectId) against
projectWithDocs.id) so selecting a new product always replaces open/active
document with that product's home; keep using getProductHomeDocument,
setOpenDocuments and setActiveDocument to perform the swap.
---
Duplicate comments:
In `@docs/ux-change-map.md`:
- Line 25: Update the P0 legend text in the priority table (the row starting
with "P0") so it becomes a complete sentence; replace the fragment "Safety /
trust / scope clarity. Should be addressed before broad release." with "P0 —
Safety, trust, and scope clarity that should be addressed before broad release."
to fix the grammar.
- Line 122: The table row describing the priority entry uses "Low risk polish"
and needs the compound adjective hyphenated; update the text to "Low-risk
polish" in the markdown table entry (the row referencing `Sidebar.tsx`) so the
phrasing is grammatically correct.
---
Nitpick comments:
In `@src/components/workspace/MainPanel.tsx`:
- Line 23: Replace the relative import of ProductHome in MainPanel.tsx with the
frontend path alias; update the import statement that currently references
'../../pages/ProductHome' to use '@/pages/ProductHome' so it follows the repo
convention for internal module imports and remains resilient to future
moves/refactors (locate the import line for ProductHome in the MainPanel
component and change the path accordingly).
- Around line 279-281: The empty-state span in MainPanel.tsx uses hard-coded
white/emerald utilities (the span with className containing border-white/10,
bg-white/[0.045], and the Sparkles using text-emerald-300); replace these with
the app’s semantic Tailwind token classes (use the HSL-based custom variables
for border, background and icon color tokens used elsewhere, e.g.,
border-[token], bg-[token], text-[token]) so the callout follows theme tokens
instead of raw colors while keeping the same layout and the Sparkles icon.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64b4ce9d-1ba4-4bde-93bc-eb0529c04cbb
⛔ Files ignored due to path filters (3)
docs/mockups/welcome-productos-refined.pngis excluded by!**/*.pngdocs/mockups/welcome-productos-refined.svgis excluded by!**/*.svgdocs/mockups/workspace-product-settings.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
docs/ux-change-map.mde2e/layout.spec.tse2e/projects.spec.tse2e/settings.spec.tssrc/components/workflow/WorkflowList.tsxsrc/components/workspace/ChatPanel.tsxsrc/components/workspace/MainPanel.tsxsrc/components/workspace/MenuBar.tsxsrc/components/workspace/ProjectFormDialog.tsxsrc/components/workspace/Sidebar.tsxsrc/pages/ProductHome.tsxsrc/pages/ProjectSettings.tsxsrc/pages/Welcome.tsxsrc/pages/Workspace.tsx
✅ Files skipped from review due to trivial changes (2)
- src/components/workspace/ProjectFormDialog.tsx
- src/components/workspace/ChatPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Welcome.tsx
AssafMiron
previously approved these changes
May 9, 2026
- Updated frontend API client (src/api/server.ts) to send 'npxCommand' instead of 'skill_command' to match backend expectation.\n- Enhanced backend route handler (node-backend/server.mjs) to support 'skill_command' as a fallback for backward compatibility.\n- Fixed 400 Bad Request error when attempting to import skills via the Import Skill dialog.
- Updated /api/settings/usage backend route to support project_id filtering - Fixed getUsageStatistics API bridge to correctly pass the project ID parameter - Enhanced Billing dashboard to automatically select and filter by the active product context - Resolved discrepancy between Sidebar project cost and App Settings global usage totals
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
node-backend/server.mjs (1)
1023-1024: ⚡ Quick winConsider clarifying the error message to reflect all accepted field names.
The error message states "npxCommand is required", but the route accepts the command from any of three fields:
npxCommand,command, orskill_command. This could confuse API consumers trying to debug failed requests.💬 Suggested improvement
- if (!npxCommand) return sendError(res, 400, 'npxCommand is required'); + if (!npxCommand) return sendError(res, 400, 'npxCommand, command, or skill_command is required');🤖 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 `@node-backend/server.mjs` around lines 1023 - 1024, The error message currently returned by the check that uses the local variable npxCommand (const npxCommand = body.npxCommand || body.command || body.skill_command) is misleading; update the sendError call to mention all accepted field names (npxCommand, command, or skill_command) so clients know which properties are valid. Locate the check that uses npxCommand and replace the message passed to sendError(res, 400, ...) with a clearer message like "one of npxCommand, command, or skill_command is required" while leaving the logic (the variable npxCommand and the truthy check) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@node-backend/server.mjs`:
- Around line 1023-1024: The error message currently returned by the check that
uses the local variable npxCommand (const npxCommand = body.npxCommand ||
body.command || body.skill_command) is misleading; update the sendError call to
mention all accepted field names (npxCommand, command, or skill_command) so
clients know which properties are valid. Locate the check that uses npxCommand
and replace the message passed to sendError(res, 400, ...) with a clearer
message like "one of npxCommand, command, or skill_command is required" while
leaving the logic (the variable npxCommand and the truthy check) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be3b9a37-09fb-4ba5-8d3f-0b8f159e0b9c
📒 Files selected for processing (8)
docs/ux-change-map.mddocs/ux-flow-map.mde2e/layout.spec.tsnode-backend/server.mjssrc/api/server.tssrc/components/workspace/ChatPanel.tsxsrc/components/workspace/MainPanel.tsxsrc/pages/Workspace.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/workspace/ChatPanel.tsx
- src/components/workspace/MainPanel.tsx
…n and project workflows
- Added high-impact 'Recommended Tasks' to Product Home dashboard - Implemented task click-to-prompt bridge to AI Copilot - Passed prompt handler through MainPanel to ProductHome - Optimized dashboard layout for command center feel
- Upgraded ConfirmationDialog with type-to-confirm and deletion scope summary - Enforced typed confirmation for Product and Workflow deletion - Added detailed deletion scope for products (files, artifacts, workflows count) - Refined ApprovalCard for stronger visual trust boundaries and premium feel - Added 'Privileged Action' badges to agent approval requests
- Updated 'Projects Directory' to 'Products Storage Path' in System Settings - Normalized descriptions in Artifact Settings to use 'Product' instead of 'Project' - Verified consistent terminology across primary UI settings components
…6 skills are loaded
…data/workflows and .workflows directories
… checks to reduce console noise
- Dispatched a new custom window event 'productos:chat-prefill-prompt' to allow users to review and edit prompts generated by recommended tasks before sending them to Copilot. - Removed duplicate backend file fetch calls in Workspace initialization to further improve switching speed.
…R D) - Added layout mode state (Split/Full/Hidden) to MainPanel for dynamic workspace sizing. - Implemented 'ToolLogBlock' in ChatPanel to collapse verbose execution logs. - Added header controls to ChatPanel for toggling layout modes. - Fixed syntax errors and unclosed tags in ChatPanel UI.
- Implemented confidence-based and date-based sorting in ArtifactList. - Normalized artifact type labels (e.g., Roadmap vs Roadmaps) in ArtifactList and Sidebar. - Updated deletion copy to 'Delete Artifact' for improved consistency. - Added descriptive subtitle to ArtifactList section.
…list view (PR E) - Replaced single-form workflow builder with a guided 4-step stepper (Details, Steps, Test, Schedule). - Enhanced WorkflowList with color-coded status indicators (Completed/Failed). - Added run history link and last-run timestamps to workflow items. - Standardized 'Create' button copy to match the guided flow.
- Deduplicate skills by ID in listSkills() - Give precedence to directory-based skills over file-based ones - Update deleteSkill() to handle both storage formats safely
- Replace native window.confirm with ConfirmationDialog for app shutdown and new chat - Modernize icons to TriangleAlert and CircleAlert (using aliases for compatibility) - Implement mandatory type-to-confirm pattern for app termination - Update Sidebar and ChatPanel to leverage standardized safety dialogs
- Update error messages and placeholders to use Product terminology for consistent UX
- Clear usage statistics when loading a new product context to prevent stale display - Improve error handling and initial state synchronization for the usage dashboard
- updated deleteProjectViaUI to handle type-to-confirm dialogs - added centralized createWorkflowViaBuilder helper to handle 4-step wizard - refactored workflows and deep_check specs to use new helpers - added robust input filling with retries and step waits
…hboard to ProductHome
…cycles and add global toggle in settings
…d in usage API - Replace direct path.join calls in workflows.mjs with shared path utilities from paths.mjs\n- Add getWorkflowsMetadataDir and getDotWorkflowsDir to paths.mjs\n- Rename ensureDirectoryStructure to initializeDirectoryStructure for consistency\n- URL-encode projectId in getUsageStatistics to handle reserved characters safely
Collaborator
Author
|
Reviewed. I cannot approve my own PR from this account, but I checked the large UX/onboarding refresh: CI is passing, no secrets found, and it removes the stray |
AssafMiron
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Screenshots / Mockups
Welcome — refined ProductOS entry
Workspace — Product settings
Existing ProductOS hero asset
Verification
npm run build✅npx playwright test e2e/layout.spec.ts✅ 5 passednpx playwright test e2e/projects.spec.ts e2e/settings.spec.ts✅ 8 passed, 1 skippednpm run test:backend✅ 44 passedSummary by CodeRabbit
Documentation
New Features
UX / Copy
Interaction
Tests