fix: home quick start widget validation #6626
Conversation
WalkthroughA new hook, Changes
Sequence Diagram(s)sequenceDiagram
participant NP as NoProjects Component
participant WS as useWorkspace Hook
participant WK as Workspace Data
NP->>WS: Invoke useWorkspace()
WS->>WK: Request active workspace information
WK-->>WS: Return workspace data
WS-->>NP: Provide activeWorkspace info
NP->>NP: Determine isWorkspaceAdmin status
NP->>NP: Check if member count >= 2
NP->>NP: Render actions with appropriate CTA states (enabled/disabled)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/core/components/home/widgets/empty-states/no-projects.tsx (3)
128-128: Consider improving the null safety check readability.The null safety check is correct but could be more readable.
-return (activeWorkspace?.total_members || 0) >= 2; +return activeWorkspace?.total_members >= 2 || false;
136-136: Maintain consistency in null safety handling.For consistency with the earlier suggestion, consider updating the null safety check here as well.
-if (storedValue?.hide || (joinedProjectIds?.length > 0 && (activeWorkspace?.total_members || 0) >= 2)) return null; +if (storedValue?.hide || (joinedProjectIds?.length > 0 && (activeWorkspace?.total_members >= 2 || false))) return null;
178-205: Consider simplifying the conditional rendering structure.The nested conditional rendering could be made more readable by extracting the link and button rendering into separate components or functions.
Example approach:
const renderCTA = (item: EmptyStateItem) => { if (isStateComplete(item.flag) || item.cta.disabled) return null; if (item.cta.link) { return ( <Link href={item.cta.link} onClick={handleLinkClick(item)} className="text-custom-primary-100 hover:text-custom-primary-200 text-sm font-medium" > {t(item.cta.text)} </Link> ); } return ( <button type="button" className="text-custom-primary-100 hover:text-custom-primary-200 text-sm font-medium" onClick={item.cta.onClick} > {t(item.cta.text)} </button> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/home/widgets/empty-states/no-projects.tsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/components/home/widgets/empty-states/no-projects.tsx (2)
15-22: LGTM! Workspace integration looks good.The addition of workspace context and admin permission checks aligns well with the PR objectives of implementing permission-based access control.
Also applies to: 34-34, 48-48
66-66: LGTM! Permission-based CTA controls look good.The disabled states for CTAs are properly implemented based on user permissions, which aligns with the PR objective of showing links based on user permissions.
Also applies to: 78-78, 90-90, 119-119
sriramveeraghanta
changed the title
* fix: Handled workspace switcher closing on click * fix: home quickstart widget
Description
This PR fixes the home's quickstart widget:
Summary by CodeRabbit