fix(sdk): polish views-new empty states, menus and add member flow#1626
fix(sdk): polish views-new empty states, menus and add member flow#1626rohanchkrabrty wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR enhances organization management UI with better load state tracking, org-level project filtering, simplified project editing, and consistent table action styling across teams and projects views. ChangesHook Infrastructure for Load State Tracking
Projects Feature Expansion
Teams View Load State and Polish
Icon Standardization and Table UX Polish
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
web/sdk/react/views-new/plans/plans-view.tsx (1)
153-158: ⚡ Quick winInconsistent icon migration compared to other empty states.
This
EmptyStateupdates tovariant="empty2"but still uses the RadixExclamationTriangleIconcomponent, while other views (billing, PAT, service-accounts) migrated to local SVG assets via theImagecomponent. Per the PR objectives, the standardization effort aims to systematically replace Radix icons with local SVG assets in empty states. Theexclamation-triangle.svgasset is already available and used in billing-view, so this can be a quick migration to align with the pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e46f3d90-cdec-4c38-b3f8-b6781089121e
⛔ Files ignored due to path filters (2)
web/sdk/react/assets/inbox-stack.svgis excluded by!**/*.svgweb/sdk/react/assets/user-minus.svgis excluded by!**/*.svg
📒 Files selected for processing (28)
web/sdk/react/hooks/useOrganizationProjects.tsweb/sdk/react/hooks/useOrganizationTeams.tsweb/sdk/react/views-new/billing/billing-view.tsxweb/sdk/react/views-new/members/components/member-columns.tsxweb/sdk/react/views-new/members/members-view.module.cssweb/sdk/react/views-new/members/members-view.tsxweb/sdk/react/views-new/pat/pat-view.tsxweb/sdk/react/views-new/plans/plans-view.tsxweb/sdk/react/views-new/projects/components/add-member-menu.tsxweb/sdk/react/views-new/projects/components/edit-project-dialog.module.cssweb/sdk/react/views-new/projects/components/edit-project-dialog.tsxweb/sdk/react/views-new/projects/components/member-columns.module.cssweb/sdk/react/views-new/projects/components/member-columns.tsxweb/sdk/react/views-new/projects/components/project-columns.module.cssweb/sdk/react/views-new/projects/components/project-columns.tsxweb/sdk/react/views-new/projects/project-details-view.module.cssweb/sdk/react/views-new/projects/project-details-view.tsxweb/sdk/react/views-new/projects/projects-view.module.cssweb/sdk/react/views-new/projects/projects-view.tsxweb/sdk/react/views-new/service-accounts/components/service-account-columns.tsxweb/sdk/react/views-new/service-accounts/service-accounts-view.tsxweb/sdk/react/views-new/teams/components/member-columns.module.cssweb/sdk/react/views-new/teams/components/member-columns.tsxweb/sdk/react/views-new/teams/components/team-columns.module.cssweb/sdk/react/views-new/teams/components/team-columns.tsxweb/sdk/react/views-new/teams/team-details-view.module.cssweb/sdk/react/views-new/teams/team-details-view.tsxweb/sdk/react/views-new/teams/teams-view.tsx
💤 Files with no reviewable changes (1)
- web/sdk/react/views-new/projects/components/edit-project-dialog.module.css
| variant="empty2" | ||
| icon={ | ||
| <Image | ||
| src={exclamationTriangleIcon as unknown as string} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the type definition of Image src prop and SVG import handling
rg -n "src\s*:\s*" --type=ts -A2 -g '*apsara*' -g '*Image*' | head -20
# Check bundler configuration for SVG handling
fd -e json -e js -e ts 'vite.config|webpack.config|next.config' --exec cat {}Repository: raystack/frontier
Length of output: 3435
🏁 Script executed:
# Find the Image component definition from apsara-v1 in the project
rg -n "interface\s+Image|type\s+Image|function\s+Image" --type=ts --type=tsx | grep -i "src\|prop" | head -20
# Check the actual billing-view file to understand the imports
cat -n web/sdk/react/views-new/billing/billing-view.tsx | head -30
# Search for other similar double type casts in the codebase
rg "as unknown as string" --type=ts --type=tsx -n | head -30
# Check if there are type definitions or other Image usages
rg -n "from.*apsara.*Image" --type=ts --type=tsx | head -20Repository: raystack/frontier
Length of output: 1695
🏁 Script executed:
# Check TypeScript configuration for SVG handling
find . -name "tsconfig*.json" -type f | head -5 | xargs cat
# Search for other Image component usages to see patterns
rg "Image" --type ts -A2 -B2 | grep -A3 "src=" | head -40
# Check other SVG imports and how they're used
rg "from.*\.svg" --type ts -B2 -A2 | head -50
# Look for similar double type casts elsewhere
rg "as unknown as" --type ts -B2 -A2 | head -40Repository: raystack/frontier
Length of output: 12410
Add TypeScript type declarations for SVG imports to eliminate the double type cast.
The pattern as unknown as string appears in 10+ files across the codebase and indicates a type mismatch between SVG imports and the Image component's src prop. Add a module declaration (e.g., src/vite-env.d.ts) to properly type SVG imports:
declare module '*.svg' {
const content: string;
export default content;
}This eliminates the need for the workaround cast throughout the codebase and provides type safety.
| .actionsCell { | ||
| visibility: hidden; | ||
| } | ||
|
|
||
| tr:hover .actionsCell { | ||
| visibility: visible; | ||
| } |
There was a problem hiding this comment.
Add keyboard accessibility support for hidden action cells.
The current CSS only reveals .actionsCell on :hover, which prevents keyboard-only users from accessing row action buttons. Elements with visibility: hidden are removed from the accessibility tree and cannot receive focus, so users navigating with Tab will not be able to access these actions.
Add :focus-within to ensure the actions are revealed when any element inside the row receives keyboard focus.
♿ Proposed fix to restore keyboard navigation
.actionsCell {
visibility: hidden;
}
-tr:hover .actionsCell {
+tr:hover .actionsCell,
+tr:focus-within .actionsCell {
visibility: visible;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .actionsCell { | |
| visibility: hidden; | |
| } | |
| tr:hover .actionsCell { | |
| visibility: visible; | |
| } | |
| .actionsCell { | |
| visibility: hidden; | |
| } | |
| tr:hover .actionsCell, | |
| tr:focus-within .actionsCell { | |
| visibility: visible; | |
| } |
| .actionsCell { | ||
| visibility: hidden; | ||
| } | ||
|
|
||
| tr:hover .actionsCell { | ||
| visibility: visible; | ||
| } |
There was a problem hiding this comment.
Hover-only action visibility blocks non-mouse users.
Row actions are hidden unless hovered, so keyboard and touch users may not be able to discover/operate them reliably.
Suggested fix
.actionsCell {
visibility: hidden;
}
-tr:hover .actionsCell {
+tr:hover .actionsCell,
+tr:focus-within .actionsCell {
visibility: visible;
}
+
+@media (hover: none) {
+ .actionsCell {
+ visibility: visible;
+ }
+}| const hasNoProjects = !isLoading && (projects?.length ?? 0) === 0; | ||
|
|
There was a problem hiding this comment.
Top-level empty-state guard blocks the org filter path.
When showOrgProjects is false and “My Projects” is empty, the early return fires before the filter renders. Users who can list org projects can get stuck on the top-level empty state and cannot switch to “All Projects”.
Suggested fix
- const hasNoProjects = !isLoading && (projects?.length ?? 0) === 0;
+ const hasNoProjects =
+ !isLoading &&
+ (projects?.length ?? 0) === 0 &&
+ (!canListOrgProjects || showOrgProjects);Also applies to: 142-177
| <AddMemberMenuContent | ||
| projectId={payload.projectId} | ||
| canUpdateProject={payload.canUpdate} | ||
| members={[]} | ||
| refetch={refetch} |
There was a problem hiding this comment.
members={[]} makes existing project users appear as addable.
This bypasses the duplicate filter in AddMemberMenuContent, so existing members are shown in “Add a member”. That causes redundant writes and can potentially reset roles to viewer via setProjectMemberRole.
| .actionsCell { | ||
| visibility: hidden; | ||
| } | ||
|
|
||
| tr:hover .actionsCell { | ||
| visibility: visible; | ||
| } |
There was a problem hiding this comment.
Hover-only reveal blocks non-mouse access to row actions.
The actions trigger is hidden unless a row is hovered, which can prevent keyboard and touch users from reaching the menu.
💡 Suggested CSS fix
.actionsCell {
visibility: hidden;
}
-tr:hover .actionsCell {
+tr:hover .actionsCell,
+tr:focus-within .actionsCell {
visibility: visible;
}
+
+@media (hover: none) {
+ .actionsCell {
+ visibility: visible;
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .actionsCell { | |
| visibility: hidden; | |
| } | |
| tr:hover .actionsCell { | |
| visibility: visible; | |
| } | |
| .actionsCell { | |
| visibility: hidden; | |
| } | |
| tr:hover .actionsCell, | |
| tr:focus-within .actionsCell { | |
| visibility: visible; | |
| } | |
| `@media` (hover: none) { | |
| .actionsCell { | |
| visibility: visible; | |
| } | |
| } |
Coverage Report for CI Build 26092878965Coverage remained the same at 42.472%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
|
||
| import { useMemo, useState } from 'react'; | ||
| import { ExclamationTriangleIcon, TrashIcon, UpdateIcon } from '@radix-ui/react-icons'; | ||
| import { ExclamationTriangleIcon, UpdateIcon } from '@radix-ui/react-icons'; |
There was a problem hiding this comment.
We should import exclamation icon from assets as before in billingview.tsx
There was a problem hiding this comment.
I didn't get your comment. Can you help me understand?
3 participants
Summary
DotsHorizontalIcon+IconButton size={2}, visible only on row hover.empty2design with SVG icons and accent CTA; CTA is omitted (not just disabled) when the user lacks permission.teams-view, mirroring projects-view, and a my/all filterSelectinprojects-viewwithisFetched-based initial-skeleton handling.AddMemberMenuContentand rendering it as aMenu.Submenu(no handle indirection).