-
Notifications
You must be signed in to change notification settings - Fork 556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: workspace settings page #3103
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request updates several settings-related components by refining styling, adjusting JSX structure, and simplifying navigation. The legacy navigation elements have been replaced by a new workspace-specific navigation component (WorkspaceNavbar), and key components now accept a workspace prop. In addition, new components such as CopyWorkspaceId have been introduced, and form handling for workspace name updates has been refactored. Overall, the changes reorganize UI elements and improve clarity without altering core business logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page
participant WorkspaceNavbar
participant Router
User->>Page: Load settings page with workspace info
Page->>WorkspaceNavbar: Render with workspace and activePage props
User->>WorkspaceNavbar: Click on a navigation item
WorkspaceNavbar->>Router: Navigate to selected page
sequenceDiagram
participant User
participant CopyWorkspaceId
participant Clipboard
participant Toast
User->>CopyWorkspaceId: Click copy button
CopyWorkspaceId->>Clipboard: Copy workspace ID
Clipboard-->>CopyWorkspaceId: Confirm copy action
CopyWorkspaceId->>Toast: Display success notification
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 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 (
|
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/settings/general/copy-workspace-id.tsx (1)
32-37
: Consider adding error handling for clipboard operationsWhile the current implementation works in modern browsers, clipboard operations can sometimes fail due to permissions or in certain environments.
<button type="button" onClick={() => { - navigator.clipboard.writeText(workspaceId); - toast.success("Copied to clipboard", { - description: workspaceId, - }); + navigator.clipboard.writeText(workspaceId) + .then(() => { + toast.success("Copied to clipboard", { + description: workspaceId, + }); + }) + .catch(() => { + toast.error("Failed to copy to clipboard"); + }); }} >apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (1)
90-94
: Remove unused onBlur handlerThe onBlur event handler doesn't perform any meaningful action - it returns early if the value is empty but doesn't define any action for non-empty values.
- onBlur={(e) => { - if (e.target.value === "") { - return; - } - }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/dashboard/app/(app)/settings/billing/client.tsx
(9 hunks)apps/dashboard/app/(app)/settings/billing/components/shell.tsx
(1 hunks)apps/dashboard/app/(app)/settings/billing/page.tsx
(4 hunks)apps/dashboard/app/(app)/settings/general/copy-workspace-id.tsx
(1 hunks)apps/dashboard/app/(app)/settings/general/page.tsx
(2 hunks)apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx
(4 hunks)apps/dashboard/app/(app)/settings/root-keys/page.tsx
(2 hunks)apps/dashboard/app/(app)/settings/team/page.tsx
(1 hunks)apps/dashboard/app/(app)/settings/workspace-navbar.tsx
(1 hunks)apps/dashboard/components/settings-card.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/dashboard/app/(app)/settings/root-keys/page.tsx (3)
apps/dashboard/app/(app)/settings/workspace-navbar.tsx (1)
WorkspaceNavbar
(32-79)apps/dashboard/components/page-content.tsx (1)
PageContent
(3-5)apps/dashboard/components/dashboard/page-header.tsx (1)
PageHeader
(18-50)
apps/dashboard/app/(app)/settings/general/copy-workspace-id.tsx (2)
apps/dashboard/components/settings-card.tsx (1)
SettingCard
(12-49)internal/icons/src/icons/clone.tsx (1)
Clone
(15-50)
apps/dashboard/app/(app)/settings/billing/client.tsx (1)
apps/dashboard/app/(app)/settings/billing/components/shell.tsx (1)
Shell
(7-26)
apps/dashboard/app/(app)/settings/team/page.tsx (2)
apps/dashboard/app/(app)/settings/workspace-navbar.tsx (1)
WorkspaceNavbar
(32-79)apps/dashboard/app/(app)/settings/team/client.tsx (1)
TeamPageClient
(22-98)
apps/dashboard/app/(app)/settings/billing/components/shell.tsx (2)
apps/dashboard/app/(app)/settings/workspace-navbar.tsx (1)
WorkspaceNavbar
(32-79)apps/dashboard/components/page-content.tsx (1)
PageContent
(3-5)
apps/dashboard/app/(app)/settings/general/page.tsx (4)
apps/dashboard/app/(app)/settings/workspace-navbar.tsx (1)
WorkspaceNavbar
(32-79)apps/dashboard/components/page-content.tsx (1)
PageContent
(3-5)apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (1)
UpdateWorkspaceName
(28-118)apps/dashboard/app/(app)/settings/general/copy-workspace-id.tsx (1)
CopyWorkspaceId
(8-45)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Packages / Test ./apps/dashboard
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (33)
apps/dashboard/components/settings-card.tsx (2)
4-4
: Improved component flexibility!Enhancing the
title
prop type to accept React nodes in addition to strings provides more flexibility. This change allows for complex content in titles, such as the combination of text and icons or other interactive elements.
43-43
: LGTM - Class order changeReordering the CSS classes from "text-accent-12 font-medium" to "font-medium text-accent-12" maintains the same visual appearance since Tailwind classes don't have order-dependent behavior. This change likely aligns with a team-wide style standard.
apps/dashboard/app/(app)/settings/general/copy-workspace-id.tsx (1)
8-45
: Well-structured component implementationThe new
CopyWorkspaceId
component effectively encapsulates the workspace ID display and copy functionality. It appropriately leverages the enhancedSettingCard
component, making good use of React nodes for both title and description to achieve the desired layout.apps/dashboard/app/(app)/settings/root-keys/page.tsx (2)
35-38
: Improved navigation consistencyReplacing the previous navigation component with
WorkspaceNavbar
enhances consistency across the application's settings pages. The component properly receives the workspace context and active page information.
45-45
: LGTM - Class reorderingThe reordering of CSS classes from "mb-20 grid grid-cols-1 gap-8 w-full" to "grid w-full grid-cols-1 gap-8 mb-20" maintains the same visual appearance since Tailwind classes don't have order-dependent behavior.
apps/dashboard/app/(app)/settings/billing/client.tsx (3)
86-86
: Improved component context with workspace propPassing the workspace prop to the Shell component aligns with the application-wide refactoring to provide components with workspace context. This change enables more cohesive navigation and workspace-specific functionality.
117-124
: LGTM - Consistent class reorderingThe class reorderings throughout this section (and elsewhere in the file) are consistent with the style changes seen in other files. These changes don't affect functionality but help maintain a consistent style convention across the codebase.
236-237
: LGTM - Link styling adjustmentsThe reordering of "text-info-11 underline" to "underline text-info-11" is consistent with the style standards being applied elsewhere in the code.
apps/dashboard/app/(app)/settings/billing/page.tsx (4)
11-11
: LGTM: Importing the new WorkspaceNavbar componentThe import statement correctly references the new WorkspaceNavbar component from the parent directory.
35-44
: UI improvement for the error stateGood enhancement to the error state UI. Now when Stripe is not configured, users still see the consistent workspace navigation rather than just an error message, providing better context and navigation options.
77-77
: Appropriate prop update for Shell componentThe Shell component now correctly receives the workspace information it needs for the new navigation structure.
118-118
: Minor style adjustmentSimple class order adjustment that maintains the same styling functionality.
apps/dashboard/app/(app)/settings/general/update-workspace-name.tsx (5)
3-4
: LGTM: Updated imports for the UI redesignThe imports have been appropriately updated to include the new SettingCard and Form-related components needed for the UI redesign.
18-18
: Improved field namingRenaming from
name
toworkspaceName
enhances clarity about what the field represents while maintaining the same validation requirements.
38-38
: Consistent field renaming in default valuesThe default values have been correctly updated to use
workspaceName
to maintain consistency with the schema update.
54-54
: More explicit submission object structureThe submission function now uses a more explicit object structure, clearly specifying which properties are being sent to the API.
59-114
: Improved UI with SettingCard componentThe UI redesign with SettingCard provides better context and usability. The improved button validation logic (disabling when appropriate) enhances user experience.
apps/dashboard/app/(app)/settings/team/page.tsx (5)
4-4
: LGTM: Importing the WorkspaceNavbar componentThe import statement correctly references the WorkspaceNavbar component, consistent with other files in this PR.
11-11
: Improved variable namingRenaming from
ws
toworkspace
enhances code readability by using a more descriptive variable name.
16-16
: Consistent variable usageThe team property access has been updated to use the renamed
workspace
variable.
18-34
: Better error handling with conditional renderingGood improvement to handle the case when a workspace isn't found, providing a clear fallback message to the user instead of potentially crashing.
20-23
: Consistent navigation pattern with WorkspaceNavbarThe navigation has been updated to use the new WorkspaceNavbar component, matching the new navigation pattern implemented across settings pages.
apps/dashboard/app/(app)/settings/workspace-navbar.tsx (4)
1-8
: LGTM: Well-structured component importsThe imports are organized logically, separating client directive, component imports, and UI elements.
9-30
: Good navigation data structureThe settings navigation data is well-organized in a clear, maintainable array structure that makes it easy to add, remove or modify navigation items in the future.
32-44
: Clear component interface with TypeScript propsThe WorkspaceNavbar component has a well-defined interface with TypeScript types that clearly communicate what data the component expects.
45-78
: Well-implemented navigation with advanced featuresThe component implementation effectively combines several UI elements:
- Breadcrumb navigation for context
- QuickNavPopover for efficient navigation between settings
- Workspace ID display with copy functionality
This provides a consistent, user-friendly navigation experience across settings pages.
apps/dashboard/app/(app)/settings/billing/components/shell.tsx (3)
5-13
: Good implementation of the new WorkspaceNavbar patternThe code now uses the standardized WorkspaceNavbar component, which creates a consistent navigation experience across workspace settings pages. The implementation correctly passes both the workspace object and the appropriate activePage props.
7-10
: Well-structured prop typingGood job on properly typing the workspace prop with explicit shape (
{ id: string; name: string }
). This ensures type safety and makes the component contract clearer for other developers.
15-18
: Minor styling adjustments maintain layout consistencyThe subtle adjustments to padding and spacing in the layout container help maintain visual consistency while accommodating the new navigation component.
apps/dashboard/app/(app)/settings/general/page.tsx (4)
2-7
: Good modularization of importsThe import changes reflect the improved component organization, with separate components for specific functionality (Separator, WorkspaceNavbar, CopyWorkspaceId) instead of generic UI components.
28-34
: Clean implementation of consistent workspace navigationThe WorkspaceNavbar implementation aligns with the pattern used in other settings pages, creating a unified navigation experience. The section header with proper styling helps establish visual hierarchy.
37-38
: Good use of semantic components for visual separationUsing the Separator component before the CopyWorkspaceId creates clear visual grouping, which improves the UX by distinguishing between different functional areas of the settings page.
30-31
: Responsive layout considerationsThe container uses
lg:w-[760px]
for larger screens while remaining flexible on smaller screens. This is a good practice for maintaining a responsive design.
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Navigation functions like it should
Looks match the Figma on other settings pages
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Refactor / Style