Repository navigation
feat(mosaic): Connected accounts wire up - #9946
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 1a919ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. 8 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 WalkthroughWalkthroughThe change adds connected-account projection, OAuth connect, reconnect, reauthorization, and removal actions. It adds pending-state and error handling, updates the profile panel to accept connected-account and Web3-wallet slots, and adds a live connected-accounts page. Tests and FAPI helpers cover account flows, error handling, and profile-panel behavior. Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~50 minutes Merge Risk: 🔵 Low · up to The connected-accounts changes have a bounded issue in the profile-panel example: users copying it will encounter an invalid Hook call. Move the Hook into a component before considering that example ready. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
austincalvelage
changed the title
a56d1d6 to
da472c4
Compare
2a5df4c to
a73729f
Compare
a73729f to
a0df666
Compare
4fbd075 to
1deb57a
Compare
34e83f5 to
4d50c80
Compare
There was a problem hiding this comment.
A lot of the helper/util behavior here is straight up duplication (with small tweaks) from the existing feature. I wonder if we should move the helpers to the shared package and reuse them across both to make it easier to keep in sync? Or if that just complicates things and creates dependencies between the two we don't want?
This isn't so much about this PR specifically, just thinking out loud about our general approach.
Pinging @alexcarpenter for thoughts too.
There was a problem hiding this comment.
Added a TODO. Similar thought in the Web3 wallet todos
There was a problem hiding this comment.
Only if you think it makes sense, and only the part you think makes sense. 😄 There's nothing wrong with a bit of duplication either for things that could drift.
Maybe a good guideline is, if it's business logic we try to de-duplicate it since we will want to keep that in sync, if it's UI-related (how we show data), it should stay duplicated even if it's similar today?
| return; | ||
| } | ||
|
|
||
| await router.navigate(url.href); |
There was a problem hiding this comment.
Reading this makes me wonder.. Our framework hacks to make navigate awaitable are extremely ugly and I'm scared they might break in the future. setActive is by far the hardest place to tackle, but I wonder if we should stop relying on awaiting navigations in Mosaic to at least not make the problem worse? We could override the type from useMosaicRouter to not return a promise.
We would need to find a better pattern before we just go do that ofc, which probably hinges on the overall routing story, but I think it's a good goal.
WDYT @alexcarpenter?
There was a problem hiding this comment.
The model+controller is honestly so much cleaner than I thought they would be, nice!
This branch was successfully deployed
3 participants
Description
Wire the Mosaic connected accounts section to Clerk so users can connect social providers, reconnect or retry failed accounts, and remove accounts through the existing Confirmation dialog. Preserve enterprise account restrictions, OAuth redirects and popup flows, API error messages, and focus restoration after removal. Add a live connected accounts page in Swingset.
The model owns Clerk access and returns loading, hidden, or ready state. Provider choices use the canonical settings strategy order, including custom providers. A shared recovery decision determines both the row status and the action to run. Actions recheck the signed-in user and current account after redirect preparation, and report an unavailable error when recovery is no longer possible. One action wrapper translates errors while preserving internal error codes and causes.
Separate connected-account and connect-provider rows give each view its own data and actions. The profile panel accepts connected-account and wallet sections as ReactNode slots, with a shared title ref for focus restoration.
Session reverification UI is deferred. Requests that require reverification display the API error.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change