-
-
Notifications
You must be signed in to change notification settings - Fork 913
feat(webapp): add GitHub onboarding flow to empty Tasks and Deployments pages #2775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ts pages - Create new resource route for GitHub settings management with loader and actions - Add GitHubSettingsPresenter to fetch connected repos and installations - Implement GitHubSettingsPanel component for reusable GitHub configuration UI - Refactor project settings page to use shared GitHubSettingsPanel component - Integrate GitHub connection flow into empty state onboarding for Tasks and Deployments - Add support for GitHub repo connection, disconnection, and branch tracking settings - Include redirect URL support for seamless navigation after GitHub actions - Remove duplicate GitHub connection code from project settings route
|
WalkthroughReplaces duplicated deployment onboarding UIs with a shared DeploymentOnboardingSteps component and adds a tabbed onboarding flow using ClientTabs and GitHubSettingsPanel. Refactors ClientTabs and Tabs to support a Variants type (segmented, underline, pipe-divider) and context-driven behavior, adjusting component props and removing ClientTabsWithUnderline. Adds a GitHubSettingsPresenter to aggregate GitHub settings server-side and introduces a new GitHub resource route with loaders, actions, forms, and UI (ConnectGitHubRepoModal, ConnectedGitHubRepoForm, GitHubSettingsPanel). Settings and deployment routes and several storybook examples updated for the new components and styles. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,css,scss}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)apps/webapp/app/components/primitives/Tabs.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
🔇 Additional comments (6)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/primitives/Tabs.tsx (1)
200-210: Text color conditional has no effect.Both branches of the ternary apply the same class
"text-text-bright", so theisActivestate doesn't change the text appearance. This looks like an oversight.<span className={cn( "text-sm transition duration-200", - isActive ? "text-text-bright" : "text-text-bright" + isActive ? "text-text-bright" : "text-text-dimmed hover:text-text-bright" )} >
🧹 Nitpick comments (5)
apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (2)
1-5: Remove unused importerr.The
errfunction from neverthrow is imported but never used in this file.-import { err, fromPromise, ok, ResultAsync } from "neverthrow"; +import { fromPromise, ok, ResultAsync } from "neverthrow";
13-23: Consider wrapping the early return inResultAsyncfor type consistency.The
call()method returnsResult<...>when GitHub is disabled (line 17) butResultAsync<...>otherwise (line 126). While both can be awaited, wrapping the early return maintains consistent typing for consumers.if (!githubAppEnabled) { - return ok({ + return ResultAsync.fromSafePromise(Promise.resolve({ enabled: false, connectedRepository: undefined, installations: undefined, isPreviewEnvironmentEnabled: undefined, - }); + })); }apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (2)
823-825: Addfetcherto the dependency array or use a stable reference.The
fetcher.loadfunction should be stable, but ESLint's exhaustive-deps rule will flag this. Consider usingfetcheror extracting the load call.useEffect(() => { fetcher.load(gitHubResourcePath(organizationSlug, projectSlug, environmentSlug)); - }, [organizationSlug, projectSlug, environmentSlug]); + }, [organizationSlug, projectSlug, environmentSlug, fetcher]);Alternatively, if
fetchercauses re-fetching issues, you can disable the lint rule with a comment explaining the intentional omission.
869-873: Remove redundant condition check.At this point in the code,
data.connectedRepositoryis guaranteed to be falsy (we're in the else branch of line 845), so the condition on line 869 is always true.return ( <div className="flex flex-col gap-2"> <GitHubConnectionPrompt gitHubAppInstallations={data.installations ?? []} organizationSlug={organizationSlug} projectSlug={projectSlug} environmentSlug={environmentSlug} redirectUrl={effectiveRedirectUrl} /> - {!data.connectedRepository && ( - <Hint> - Connect your GitHub repository to automatically deploy your changes. - </Hint> - )} + <Hint> + Connect your GitHub repository to automatically deploy your changes. + </Hint> </div> - );apps/webapp/app/components/primitives/ClientTabs.tsx (1)
63-81: Consider aligning default variant with Tabs component.
ClientTabsListdefaults to"pipe-divider"(line 68), whileTabsandTabContainerdefault to"underline". If intentional, this is fine, but aligning defaults would reduce surprise when switching between the two tab implementations.->(({ className, variant = "pipe-divider", ...props }, ref) => { +>(({ className, variant = "underline", ...props }, ref) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/webapp/app/components/BlankStatePanels.tsx(5 hunks)apps/webapp/app/components/primitives/ClientTabs.tsx(2 hunks)apps/webapp/app/components/primitives/Tabs.tsx(3 hunks)apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(5 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx(1 hunks)apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx(1 hunks)apps/webapp/app/routes/storybook.tabs/route.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.tsapps/webapp/app/components/primitives/Tabs.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsxapps/webapp/app/routes/storybook.tabs/route.tsxapps/webapp/app/components/BlankStatePanels.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/components/primitives/ClientTabs.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx
📚 Learning: 2025-12-08T15:19:56.801Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.801Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts
📚 Learning: 2025-09-02T11:27:36.336Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.336Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-08-14T12:13:20.455Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.455Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (7)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (2)
apps/webapp/app/components/layout/AppLayout.tsx (1)
MainCenteredContainer(50-64)apps/webapp/app/components/BlankStatePanels.tsx (1)
DeploymentsNoneDev(224-277)
apps/webapp/app/components/primitives/Tabs.tsx (2)
apps/webapp/app/components/primitives/ClientTabs.tsx (1)
TabsProps(200-209)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (4)
apps/webapp/app/utils/pathBuilder.ts (3)
EnvironmentParamSchema(26-28)githubAppInstallPath(148-152)v3ProjectSettingsPath(427-433)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
findEnvironmentBySlug(116-145)apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (1)
GitHubSettingsPresenter(12-137)apps/webapp/app/services/projectSettings.server.ts (1)
ProjectSettingsService(9-329)
apps/webapp/app/routes/storybook.tabs/route.tsx (4)
apps/webapp/app/components/primitives/Headers.tsx (1)
Header1(32-50)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)apps/webapp/app/components/primitives/Tabs.tsx (1)
Tabs(20-30)apps/webapp/app/components/primitives/ClientTabs.tsx (4)
ClientTabs(211-211)ClientTabsList(211-211)ClientTabsTrigger(211-211)ClientTabsContent(211-211)
apps/webapp/app/components/BlankStatePanels.tsx (7)
apps/webapp/app/components/environments/EnvironmentLabel.tsx (2)
EnvironmentIcon(14-48)environmentFullTitle(145-160)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton(335-401)apps/webapp/app/utils/pathBuilder.ts (2)
docsPath(510-512)v3BillingPath(487-491)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)apps/webapp/app/components/SetupCommands.tsx (1)
PackageManagerProvider(21-29)apps/webapp/app/components/primitives/TextLink.tsx (1)
TextLink(29-94)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (1)
GitHubSettingsPanel(807-877)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (2)
loader(117-142)GitHubSettingsPanel(807-877)apps/webapp/app/utils/pathBuilder.ts (1)
v3BillingPath(487-491)
apps/webapp/app/components/primitives/ClientTabs.tsx (2)
apps/webapp/app/components/primitives/Tabs.tsx (1)
Variants(8-8)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
362-368: LGTM!The width change from
max-w-mdtomax-w-proseis appropriate for the onboarding content, which contains multiple paragraphs and step instructions. This provides better readability with approximately 65 characters per line.apps/webapp/app/presenters/v3/GitHubSettingsPresenter.server.ts (1)
74-107: LGTM with a note on pagination.The query limits (
take: 200for repositories,take: 20for installations) are reasonable defaults. For organizations with many installations or repositories, consider adding ahasMoreindicator or pagination support in a future iteration if users report truncated results.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.github.tsx (4)
57-111: LGTM!Types and schemas are well-structured. The use of
typeoverinterfaceand Zod for validation aligns with the coding guidelines. The discriminated union pattern forGitHubActionSchemaenables type-safe action dispatch.
117-142: LGTM!The loader follows Remix conventions, uses proper error handling with 404/500 responses, and correctly integrates with the
GitHubSettingsPresenter.
164-309: LGTM!The action handler is well-structured with:
- Proper membership verification before mutations
- Clear separation using
ProjectSettingsService- User-friendly error messages for known error types
- Exhaustive type checking with
satisfies never
379-383: Verify: Modal auto-close logic may be unreachable.The action handler always redirects on success (lines 210-216), so
lastSubmissionwon't contain a success response—the page will navigate away. Additionally, the conform/zodparseresult structure doesn't include asuccessproperty in this form.This effect may be dead code. If auto-closing is needed for non-redirect scenarios, consider checking the actual submission result shape.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
46-46: LGTM!Good refactoring to import the shared
GitHubSettingsPanelcomponent, consolidating GitHub settings UI into a single reusable module.
414-426: LGTM!The integration of
GitHubSettingsPanelis clean. The component handles its own data fetching via a fetcher, and thebillingPathprop is correctly constructed usingv3BillingPath. This removes duplicate GitHub-related code from the settings route.apps/webapp/app/routes/storybook.tabs.$tabNumber/route.tsx (1)
3-10: LGTM!Minor styling update to the storybook tab content display. The centered layout with background is appropriate for demonstrating tab content.
apps/webapp/app/routes/storybook.tabs/route.tsx (1)
12-188: LGTM! Storybook demo showcases all tab variants effectively.The two-column layout clearly demonstrates the difference between URL-based
Tabsand state-basedClientTabsacross all three variants. The multiple<Outlet />components (lines 30, 43, 56) will all render the same child route content, which is appropriate for this demonstration context.apps/webapp/app/components/BlankStatePanels.tsx (4)
55-61: LGTM! Clean imports for the new tabbed onboarding UI.The ClientTabs primitives and GitHubSettingsPanel imports are correctly added to support the new deployment onboarding flow.
102-104: Good consolidation of deployment onboarding.Delegating to
DeploymentOnboardingStepsreduces duplication and centralizes the onboarding logic.
224-277: LGTM! DeploymentsNoneDev refactored with cleaner layout.The new layout with
EnvironmentSelectorand doc/AI assistant links provides a streamlined experience for the development environment.
594-688: Well-structured tabbed deployment onboarding component.The
DeploymentOnboardingStepscomponent effectively consolidates the three deployment methods (GitHub, CLI, GitHub Actions) into a tabbed interface. TheGitHubSettingsPanelintegration at lines 652-657 correctly wires up the GitHub connection flow.One minor observation: the
billingPathprop uses{ slug: organization.slug }rather than passingorganizationdirectly. This is fine sincev3BillingPathonly needs the slug, but for consistency with other usages, you could passorganizationdirectly ifOrgForPathtype is satisfied.apps/webapp/app/components/primitives/Tabs.tsx (3)
8-9: Good use of string union type for variants.Using
type Variants = "underline" | "pipe-divider" | "segmented"follows the coding guideline to prefer string unions over enums. As per coding guidelines.
32-58: LGTM! TabContainer variant rendering is clean.The conditional rendering for each variant provides distinct styling while maintaining a consistent API.
71-161: LGTM! TabLink variant implementations with smooth animations.The variant-specific rendering with motion animations provides a polished UI experience. The spring configurations are appropriate for each variant's visual style.
apps/webapp/app/components/primitives/ClientTabs.tsx (3)
9-17: LGTM! Context pattern for tracking active tab value.The context approach enables
ClientTabsTriggerto determine its active state without prop drilling, which is cleaner than relying solely on Radix's internal state.
22-60: Well-implemented controlled/uncontrolled pattern.The component correctly handles both controlled (
valueprop) and uncontrolled (defaultValueprop) modes. TheuseEffectsyncs external value changes, andhandleValueChangeproperly calls the callback while managing internal state only when uncontrolled.
84-182: LGTM! Comprehensive variant rendering in ClientTabsTrigger.The trigger correctly uses context to determine active state and provides appropriate rendering for all three variants. The layoutId handling gracefully falls back to static elements when not provided.
✅ Checklist
Testing
Set up the local github application, and tested its connection with trigger.dev
Checked:
Changelog
Screenshots
GH-connections.mp4
🐐