Extract shared admin UI primitives - #649
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds shared account-management navigation, table, notice, and clipboard primitives, then updates the app shell and several account/admin routes to use them instead of local implementations. ChangesShared account-management components and adoption
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
kentcdodds
marked this pull request as ready for review
July 6, 2026 22:52
Contributor
|
🔎 Preview deployed: https://kody-pr-649.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/client/routes/account-remote-connectors.tsx (1)
610-649: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winConnector selection isn't blocked during save/delete, risking editor-state clobbering.
AccountManagementListItemButtonhere has nodisabledprop andonClickcallsselectConnectorunconditionally. Compare with the sibling refactor inaccount-package-invocation-tokens.tsx, which passesdisabled={isMutating}and guards the click handler (if (isMutating) return) for the identical shared component.Without this guard, selecting a different connector while
saveConnector/deleteConnectoris in flight can corrupt state:
deleteConnectorunconditionally doeseditorState = createEmptyEditorState()after its await resolves, wiping out whatever connector the user selected in the meantime.saveConnector'sapplyPayloadreads the (possibly since-changed) globaleditorState.idto decide what to refresh, showing the wrong connector's data after a mid-flight selection change.🔒 Proposed fix: guard selection during mutation
<AccountManagementListItemButton active={editorState.id === connector.id} - onClick={() => selectConnector(connector)} + disabled={isMutating} + onClick={() => { + if (isMutating) return + selectConnector(connector) + }} >🤖 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 `@packages/worker/client/routes/account-remote-connectors.tsx` around lines 610 - 649, Selection in the connector list must be blocked while save/delete mutations are in flight, because `selectConnector` can clobber `editorState` mid-operation. Update the `AccountManagementListItemButton` usage in `account-remote-connectors.tsx` to accept the same `disabled={isMutating}` pattern used in `account-package-invocation-tokens.tsx`, and add an early return in the click handler when mutating. Make sure the guard applies around the `connectors.map(...)` item button and uses the existing `isMutating` state so `saveConnector` and `deleteConnector` can complete without the user switching entries.
🧹 Nitpick comments (2)
packages/worker/client/routes/account-management-components.tsx (2)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exporting
AccountManagementLinkNavItem.The type isn't exported, so consumers building
itemsarrays (e.g.admin-community-reports.tsx) can only rely on structural typing rather than importing the shared shape. Since this is explicitly a "shared primitive" meant for reuse across surfaces, exporting it would make the contract explicit for future consumers.🤖 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 `@packages/worker/client/routes/account-management-components.tsx` around lines 21 - 26, Export the shared AccountManagementLinkNavItem type so other surfaces can import the explicit contract instead of depending on structural typing. Update the type declaration in account-management-components.tsx to be exported, and keep its shape aligned with the existing usage in the account management link components and any consumers like admin-community-reports.tsx.
138-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueActive-state styling duplicates hover colors already in
getSecondaryButtonCss.The inline
borderColor: colors.primary/backgroundColor: colors.primarySoftestoverride foritem.activeduplicates the same color pair already used ingetSecondaryButtonCss's:hoverstate. Not a bug, but worth noting since this file is meant to centralize such styling.🤖 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 `@packages/worker/client/routes/account-management-components.tsx` around lines 138 - 144, The active-state inline styling in the account management component duplicates the same primary/softest color pair already defined in getSecondaryButtonCss hover styles. Update the item.active branch in account-management-components.tsx to reuse the shared button styling logic instead of repeating borderColor/backgroundColor overrides, keeping the active appearance consistent with the centralized styling helper.
🤖 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.
Outside diff comments:
In `@packages/worker/client/routes/account-remote-connectors.tsx`:
- Around line 610-649: Selection in the connector list must be blocked while
save/delete mutations are in flight, because `selectConnector` can clobber
`editorState` mid-operation. Update the `AccountManagementListItemButton` usage
in `account-remote-connectors.tsx` to accept the same `disabled={isMutating}`
pattern used in `account-package-invocation-tokens.tsx`, and add an early return
in the click handler when mutating. Make sure the guard applies around the
`connectors.map(...)` item button and uses the existing `isMutating` state so
`saveConnector` and `deleteConnector` can complete without the user switching
entries.
---
Nitpick comments:
In `@packages/worker/client/routes/account-management-components.tsx`:
- Around line 21-26: Export the shared AccountManagementLinkNavItem type so
other surfaces can import the explicit contract instead of depending on
structural typing. Update the type declaration in
account-management-components.tsx to be exported, and keep its shape aligned
with the existing usage in the account management link components and any
consumers like admin-community-reports.tsx.
- Around line 138-144: The active-state inline styling in the account management
component duplicates the same primary/softest color pair already defined in
getSecondaryButtonCss hover styles. Update the item.active branch in
account-management-components.tsx to reuse the shared button styling logic
instead of repeating borderColor/backgroundColor overrides, keeping the active
appearance consistent with the centralized styling helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 084546e2-caee-4aea-8b7b-a8600045b8e1
📒 Files selected for processing (9)
packages/worker/client/app.tsxpackages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/account-package-invocation-tokens.tsxpackages/worker/client/routes/account-remote-connectors.tsxpackages/worker/client/routes/account.tsxpackages/worker/client/routes/admin-community-reports.tsxpackages/worker/client/routes/admin-invites.tsxpackages/worker/client/routes/admin-system-email.tsxpackages/worker/client/routes/admin-usage.tsx
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AccountManagementLinkNavfor shared pill-style navigation.Walkthrough
Latest account UI primitive reuse:
Final Remote Connectors state:
Earlier admin reuse cleanup:
Testing
git push -u origin cursor/admin-ui-reuse-cleanup-6213(latest pre-push ran typecheck, unit tests, client build, and full Playwright E2E: 186 unit files / 586 tests passed; 6 E2E passed)System recap — composes existing primitives (low risk)
Mode: recap · Base:
main@07d14bf· Head:9fb1eb3Classification: composes — no primitives added or changed; this PR reuses existing app/account/admin UI primitives across more routes.
Primitives touched
app-uiSystem map
Before / after
AccountManagementLayout,AccountManagementSidebar,AccountManagementList, andAccountManagementListItemButton.AccountManagementShell,AccountManagementHeader, andAccountManagementMessage.primaryLinkCss; copy actions bypassed shared clipboard fallback.AccountManagementLinkNav.noticeCardCss.Summary by CodeRabbit
New Features
Bug Fixes
Style
…).