fix: DialogTitle accessibility warning and async wisp error in AuthPr…#65
Conversation
…ovider - Replace bindSupabase with manual wisp.identify/reset in auth state listener to catch async errors (wisp not initialized was throwing unhandled promise rejection) - Add visually hidden DialogTitle to CommandDialog to fix Radix accessibility warning
|
@Coder-soft is attempting to deploy a commit to the yamura3's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 53 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe command dialog gains an accessible hidden title. AuthProvider now directly synchronizes Wisp identity with initial sessions, sign-ins, and sign-outs while tolerating uninitialized Wisp state. ChangesCommand dialog accessibility
Auth analytics integration
Estimated code review effort: 2 (Simple) | ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
Greptile SummaryThis PR updates the command dialog accessibility label and Wisp auth identity handling.
Confidence Score: 4/5The Wisp auth path needs a small fix before merging.
src/providers/AuthProvider.tsx
|
| Filename | Overview |
|---|---|
| src/components/ui/command.tsx | Adds a hidden DialogTitle to satisfy the dialog accessible-name requirement. |
| src/providers/AuthProvider.tsx | Replaces the Supabase Wisp binding with manual identify/reset calls, but the new error handling can still miss async failures. |
Reviews (1): Last reviewed commit: "fix: DialogTitle accessibility warning a..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function tryIdentify(userId: string) { | ||
| try { wisp.identify(userId); } catch { /* wisp not initialized */ } | ||
| } | ||
| function tryReset() { | ||
| try { wisp.reset(); } catch { /* wisp not initialized */ } | ||
| } |
There was a problem hiding this comment.
When wisp.identify() or wisp.reset() returns a rejected Promise because Wisp is not initialized, this synchronous try/catch does not catch it. The auth listener can still produce the unhandled promise rejection this change is meant to prevent.
| function tryIdentify(userId: string) { | |
| try { wisp.identify(userId); } catch { /* wisp not initialized */ } | |
| } | |
| function tryReset() { | |
| try { wisp.reset(); } catch { /* wisp not initialized */ } | |
| } | |
| function tryIdentify(userId: string) { | |
| Promise.resolve() | |
| .then(() => wisp.identify(userId)) | |
| .catch(() => { /* wisp not initialized */ }); | |
| } | |
| function tryReset() { | |
| Promise.resolve() | |
| .then(() => wisp.reset()) | |
| .catch(() => { /* wisp not initialized */ }); | |
| } |
| if (event === "SIGNED_IN" && session?.user.id) { | ||
| tryIdentify(session.user.id); | ||
| } else if (event === "SIGNED_OUT") { | ||
| tryReset(); |
There was a problem hiding this comment.
Supabase auth changes that keep a user signed in, such as token refresh or user update events, still carry the current session but no longer call Wisp identify. If Wisp uses the current auth session or user metadata for its identity state, those updates can leave analytics tied to stale user data until a full sign-out and sign-in happens.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
- AdBlockDetector: remove dead isBlocked state (set but never read) - Showcase: remove redundant aspect-video from inner font/json/document previews - ResourcesHub: replace useState/useEffect tab with useSearchParams for URL sync - useUserFavorites: add isSchemaReady to query enabled condition to prevent pointless refetches - useHeartedResources: use heartedResources state instead of localStorage read in isHearted/toggleHeart
This reverts commit b30732d.
2 participants
…ovider
Summary by CodeRabbit
Accessibility
Bug Fixes