feat: redesign integrations page with Appwrite-style provider grid an… - #366
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughOAuth provider management is consolidated in IntegrationsSettings.jsx with searchable provider cards and an editing modal. Auth.jsx removes its configuration flow and links to Integrations settings. Mail and storage settings receive related UI updates. ChangesOAuth settings consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant IntegrationsSettings
participant AuthProvidersAPI
User->>IntegrationsSettings: Select OAuth provider
IntegrationsSettings->>IntegrationsSettings: Edit provider settings
IntegrationsSettings->>AuthProvidersAPI: PATCH provider configuration
AuthProvidersAPI-->>IntegrationsSettings: Return updated configuration
IntegrationsSettings-->>User: Show updated provider status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/web-dashboard/src/pages/Auth.jsx (1)
407-416: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate documentation for the relocated OAuth configuration flow.
OAuth provider setup moved from the Auth page to Settings → Integrations; repository docs and in-dashboard docs that reference the old location need updating. Want me to open an issue to track this?
As per coding guidelines: "When a user-facing flow changes, update both repository documentation and in-dashboard documentation."
🤖 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 `@apps/web-dashboard/src/pages/Auth.jsx` around lines 407 - 416, Update the repository documentation and in-dashboard documentation references to OAuth provider setup so they point to Settings → Integrations instead of the Auth page. Locate references associated with the “OAuth2 & Social Providers” section and preserve the existing provider configuration guidance while correcting the navigation path.Source: Coding guidelines
apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsx (2)
379-398: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd dialog semantics and Escape handling to the modal.
The overlay lacks
role="dialog"/aria-modal/labelling and does not close on Escape, unlike typical modal behavior. Focus also stays on the card behind it when opened.♻️ Suggested additions
+ <div + className="modal-overlay" + role="dialog" + aria-modal="true" + aria-label={`${selectedProviderModal === 'github' ? 'GitHub' : 'Google'} OAuth2 settings`}Plus an effect while the modal is open:
useEffect(() => { if (!selectedProviderModal) return; const onKey = (e) => { if (e.key === 'Escape') setSelectedProviderModal(null); }; window.addEventListener('keydown', onKey); return () => window.removeEventListener('keydown', onKey); }, [selectedProviderModal]);🤖 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 `@apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsx` around lines 379 - 398, Update the selectedProviderModal modal flow to add role="dialog", aria-modal="true", and an accessible label referencing the modal heading. Add a useEffect keyed to selectedProviderModal that closes the modal on Escape and removes the keydown listener on cleanup; also move focus into the opened modal, such as focusing the existing close button or dialog container.
273-286: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated section headings from nested
SettingsCard.
DatabaseConfigFormrenders its ownSettingsCard title="Database (MongoDB)"andStorageConfigFormrenderstitle="Storage (BYOS)", so the new outer cards/headers ("MongoDB", "Storage Engines (BYOS)") duplicate them visually. Consider passing a flag to suppress the inner card chrome, or dropping the outer wrapper.Also applies to: 369-376
🤖 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 `@apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsx` around lines 273 - 286, Remove the redundant outer card and header wrappers around DatabaseConfigForm and StorageConfigForm, or pass an explicit prop that suppresses their inner SettingsCard chrome. Ensure each section displays only one heading, preserving the existing DatabaseConfigForm title “Database (MongoDB)” and StorageConfigForm title “Storage (BYOS)”.
🤖 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 `@apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsx`:
- Around line 222-239: Update the provider card div in the integrations settings
rendering to use the established clickable-card accessibility pattern: add an
appropriate button role, make enabled cards keyboard-focusable, and handle
Enter/Space in onKeyDown by invoking openOAuthModal while preserving disabled
behavior. Rename the invalid justify style key to justifyContent so the spacing
is applied.
- Around line 140-144: Normalize auth providers before updating state or project
data so every provider, especially github, has clientSecret defaulted to an
empty string when missing. Apply this consistently to the patch response handled
near setAuthProviders and the project-load effect, preserving existing
clientSecret values.
- Around line 119-138: Update handleSaveOAuthModal to validate the selected
provider before issuing the PATCH: when updatedProviderData.enabled is true,
require a non-empty clientId and either a newly supplied clientSecret or an
existing stored secret. If credentials are missing, show the established
validation feedback, stop the save flow, and avoid setting or retaining the
PATCH request.
---
Nitpick comments:
In `@apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsx`:
- Around line 379-398: Update the selectedProviderModal modal flow to add
role="dialog", aria-modal="true", and an accessible label referencing the modal
heading. Add a useEffect keyed to selectedProviderModal that closes the modal on
Escape and removes the keydown listener on cleanup; also move focus into the
opened modal, such as focusing the existing close button or dialog container.
- Around line 273-286: Remove the redundant outer card and header wrappers
around DatabaseConfigForm and StorageConfigForm, or pass an explicit prop that
suppresses their inner SettingsCard chrome. Ensure each section displays only
one heading, preserving the existing DatabaseConfigForm title “Database
(MongoDB)” and StorageConfigForm title “Storage (BYOS)”.
In `@apps/web-dashboard/src/pages/Auth.jsx`:
- Around line 407-416: Update the repository documentation and in-dashboard
documentation references to OAuth provider setup so they point to Settings →
Integrations instead of the Auth page. Locate references associated with the
“OAuth2 & Social Providers” section and preserve the existing provider
configuration guidance while correcting the navigation path.
🪄 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 Plus
Run ID: 20b80ead-3805-47e2-9097-3771e6f069de
📒 Files selected for processing (2)
apps/web-dashboard/src/components/Settings/IntegrationsSettings.jsxapps/web-dashboard/src/pages/Auth.jsx
…, normalization, validation, and layout
settings page redesign
Summary by CodeRabbit
New Features
Improvements