feat: reusable core AppSidebar in toolkit and configurable top-left alpha badge - #4697
Conversation
…e-sidebar-and-alpha-badge.md, .claude/launch.json, … (78 files)
…ges/toolkit, … (18 files)
Resolve secrets UI/routes conflicts by taking main's managedHere and asRequestUser changes.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…tes/calendar, … (5 files)
|
Addressing the remaining Builder review-summary items that were not covered by the earlier inline-thread fixes:
The five inline HIGH/MEDIUM findings were fixed and replied on the prior push ( |
…ges/toolkit, … (20 files)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ges/toolkit, … (14 files)
…tes/macros (4 files)
There was a problem hiding this comment.
Builder reviewed your changes and found 5 potential issues 🔴
Review Details
Code Review Summary
PR #4697 centralizes the template sidebars around a new toolkit AppSidebar, adds the core wrapper and configurable environment badge, and migrates many applications away from bespoke sidebar implementations. The overall direction is sound: shared primitives reduce duplicated layout code and retain app-specific navigation through slots, while the badge configuration is cleanly exposed at the core boundary. I verified that the previously reported Mail content-offset issue is fixed by guarding the offset with !isMobile, and resolved that stale thread.
This is standard risk because the change affects shared UI infrastructure, package exports, authentication-adjacent environment navigation, and many application layouts.
Key Findings
- 🔴 HIGH — The toolkit package is not included in the changeset even though Core now imports its new
AppSidebarexport; publishing Core without the matching toolkit release can fail module initialization. - 🟡 MEDIUM — Several migrated sidebars lose the existing command-search footer action.
- 🟡 MEDIUM — Design editor routes no longer mark Designs active because the migration checks
/d/instead of/design/. - 🟡 MEDIUM — Signed-out/non-Builder users on beta can no longer follow the production link from the badge/auth flow.
- 🟡 MEDIUM — The Content sidebar drops its expanded header wrapper and collapsed settings control sizing.
The shared component has useful extension points and focused typechecks reported by reviewers pass. Localization and accessibility regressions are also included below where applicable.
🧪 Browser testing: Will run after this review (PR touches UI code)
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| "@agent-native/core": patch | |||
There was a problem hiding this comment.
🔴 Release the toolkit alongside the new AppSidebar export
This changeset bumps @agent-native/core, but not @agent-native/toolkit, despite adding the AppSidebar export in packages/toolkit/src/app-shell/index.ts. Core now imports that symbol from @agent-native/toolkit/app-shell; a consumer installing the patched Core with the currently published toolkit can therefore fail at module initialization. Add a patch changeset for @agent-native/toolkit so the export and Core release are published together.
There was a problem hiding this comment.
Fixed: changeset now includes a patch bump for @agent-native/toolkit alongside core/dispatch so the new AppSidebar export publishes with Core.
| brandName={APP_TITLE} | ||
| brandHref="/home" | ||
| secondaryItems={secondaryItems} | ||
| feedback={feedbackButton} |
There was a problem hiding this comment.
🟡 Preserve the command-search action in migrated sidebars
The old chat sidebar passed a searchButton to its footer actions, but the replacement AppSidebar receives only feedback and the org switcher. The new footer has no search slot or built-in command-search control, so users lose the existing command-menu/search affordance (the same migration pattern affects other templates). Preserve the existing action through footerExtras or add an explicit search slot.
There was a problem hiding this comment.
Fixed: command-search is restored via footerExtras on chat and the other migrated AppSidebar templates that previously exposed openCommandMenu (brain, assets, forms, tasks, slides, factory).
| label: t("navigation.designs"), | ||
| icon: IconPencil, | ||
| active: | ||
| location.pathname === "/home" || location.pathname.startsWith("/d/"), |
There was a problem hiding this comment.
🟡 Keep the editor route active in the Designs nav item
The active-route check was changed from the existing editor route prefix (/design/) to /d/. The Design app's router and surrounding route handling use /design/:id, so opening a design editor no longer marks Designs active in the sidebar. Restore the /design/ prefix, or support both if /d/ is intentional.
There was a problem hiding this comment.
Fixed: Designs nav active state now matches /design/ (and still accepts /d/ if used).
| className, | ||
| ); | ||
|
|
||
| if (!isBuilder) { |
There was a problem hiding this comment.
🟡 Keep the beta-to-production link available before authentication
The beta badge now returns a static status <div> whenever isBuilderIoEmployee(session?.email) is false, which includes signed-out visitors. That removes the production link from the badge/auth flow for visitors on a beta hostname, even though the environment target is still computed and the previous auth-page switcher was removed. Keep the production link available for unauthenticated visitors while restricting the internal-only popover controls.
There was a problem hiding this comment.
Fixed: non-Builder visitors get a one-click lane switch link on the badge (beta→production / production→beta). The hide/popover controls stay Builder-only.
| <div className="flex h-12 shrink-0 items-center justify-between border-b border-border px-3"> | ||
| {brandButton(false)} | ||
| </div> | ||
| {brandButton(false)} |
There was a problem hiding this comment.
🟡 Restore the expanded Content sidebar header container
The expanded render now inserts brandButton(false) directly, but brandButton returns an unstyled wrapper containing the brand button and inline environment badge. The previous parent supplied the fixed header height, padding, alignment, and bottom border, so the Content sidebar now loses its header boundary and the badge can stack below the brand. Keep an equivalent header flex container around these controls.
There was a problem hiding this comment.
Fixed: restored the expanded Content header container (h-12 + border) and made the brand/badge row a flex container so the badge sits beside the brand.
…pha-badge.md, packages/core, templates/assets, … (11 files)
…ges/toolkit, … (8 files)
…ates/analytics, … (6 files)
…ates/content, … (4 files)
1 participant
Summary
AppSidebarcomponents in@agent-native/toolkitand re-exported from@agent-native/core/client/ui, modeled directly on Shawn's Clips sidebar design.@builder.ioemail sessions while showing static status badges to external visitors.badgeTextprop oragent-native.config.tsdeployment configuration.Verification
packages/core/src/client/EnvironmentBadge.spec.ts,packages/core/src/client/EnvironmentBadge.render.spec.tsx,packages/toolkit/src/app-shell/sidebar.spec.tsx, andtemplates/slides/app/components/layout/Sidebar.test.tsxpass.pnpm guards) pass cleanly.