-
-
Notifications
You must be signed in to change notification settings - Fork 853
chore(webapp): pricing page improvements #2457
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
|
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🧹 Nitpick comments (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
1098-1110
: Nit: stabilize capitalization for “Preview branches”You toggle “preview” vs “Preview” based on count. Consider consistent casing (“Preview branches”) to avoid subtle visual jitter across tiers.
- {limits.branches.number > 0 ? "preview" : "Preview"} branches + Preview branches
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/webapp/app/components/DefinitionTooltip.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
(2 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
(24 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/components/DefinitionTooltip.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/components/DefinitionTooltip.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/components/DefinitionTooltip.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
🧬 Code graph analysis (3)
apps/webapp/app/components/DefinitionTooltip.tsx (1)
apps/webapp/app/components/primitives/Tooltip.tsx (1)
TooltipTrigger
(128-128)
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (3)
apps/webapp/app/components/layout/AppLayout.tsx (2)
AppContainer
(4-16)PageBody
(27-48)apps/webapp/app/components/BackgroundWrapper.tsx (1)
BackgroundWrapper
(6-48)apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
PricingPlans
(237-268)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (3)
apps/webapp/app/components/primitives/Buttons.tsx (1)
Button
(296-331)apps/webapp/app/utils/cn.ts (1)
cn
(77-79)apps/webapp/app/components/DefinitionTooltip.tsx (1)
DefinitionTip
(5-33)
⏰ 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 / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 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 / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/components/DefinitionTooltip.tsx (1)
17-17
: Tooltip trigger alignment change looks goodLeft-aligning the trigger improves consistency with other left-aligned inline labels; no behavioral impact.
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (2)
1-1
: Good: type-only import for LoaderFunctionArgsThis reduces emitted JS and matches Remix typing conventions.
52-70
: Verify scroll + background layering after PageBody refactorPageBody now provides the scroll container and BackgroundWrapper paints fixed-height (100vh) layers. On very tall content, below-the-fold areas rely on PageBody’s bg-charcoal-900. Please sanity-check for any visible seam at ~100vh and double-scroll on small screens.
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (6)
748-751
: Nice: extensible “children” line for add-ons across featuresThe pattern cleanly supports inline add-on pricing without duplicating list items. Prop types look correct and optional.
Also applies to: 760-760, 762-764, 767-770, 953-977, 979-991, 1007-1025, 1067-1091, 1093-1115
930-940
: Icon/text vertical alignment improvement looks gooditems-start plus the 0.5 top offset on icons fixes baseline wobble in multi-line labels.
840-841
: Indigo highlight styling: consistent and readableBorder and heading color updates align with the PR’s brand shift. Contrast remains adequate against bg.
Also applies to: 871-874
896-906
: TierLimit: left-align + w-fit on tooltip trigger is a good UX tweakPrevents full-width hover area and matches surrounding text alignment.
648-649
: Rename check complete
No instances of the old “RealtimeConneurrency” misspelling remain in the repo; no further action required.
362-362
: Secondary/medium and secondary/large variants are defined inButtons.tsx
(lines 113–114) viacreateVariant
, confirming they exist and map to the design system.
additionalConcurrency: { | ||
title: "Additional concurrency", | ||
content: "Then $50/month per 50", | ||
}, | ||
taskRun: { |
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.
Clarify “Then $… per …” copy with explicit units
Several added pricing strings omit units, which can confuse users (e.g., “per 50” of what?). Recommend making units explicit and adding “+” to indicate incremental pricing.
Apply this diff:
additionalConcurrency: {
title: "Additional concurrency",
- content: "Then $50/month per 50",
+ content: "Then +$50/month per additional 50 concurrent runs",
},
@@
additionalSchedules: {
title: "Additional schedules",
- content: "Then $10/month per 1,000",
+ content: "Then +$10/month per additional 1,000 schedules",
},
@@
additionalRealtimeConnections: {
title: "Additional Realtime connections",
- content: "Then $10/month per 100",
+ content: "Then +$10/month per additional 100 Realtime connections",
},
additionalSeats: {
title: "Additional seats",
- content: "Then $20/month per seat",
+ content: "Then +$20/month per additional seat",
},
@@
additionalBranches: {
title: "Additional branches",
- content: "Then $10/month per branch",
+ content: "Then +$10/month per additional branch",
},
Also applies to: 195-198, 209-216, 219-225
<div className="flex h-10 w-full cursor-pointer items-center justify-center rounded border border-charcoal-600 bg-tertiary px-8 text-base font-medium transition hover:border-charcoal-550 hover:bg-charcoal-600"> | ||
<span className="text-center text-text-bright">Contact us</span> | ||
</div> | ||
} | ||
></Feedback> | ||
/> | ||
</div> |
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.
Accessibility: use a semantic button for the Enterprise CTA
A styled div is not keyboard-focusable or announced by assistive tech. Use a native button (or your Button component) to avoid a11y regressions.
Apply this minimal fix:
- <div className="flex h-10 w-full cursor-pointer items-center justify-center rounded border border-charcoal-600 bg-tertiary px-8 text-base font-medium transition hover:border-charcoal-550 hover:bg-charcoal-600">
- <span className="text-center text-text-bright">Contact us</span>
- </div>
+ <button
+ type="button"
+ className="flex h-10 w-full items-center justify-center rounded border border-charcoal-600 bg-tertiary px-8 text-base font-medium text-text-bright transition hover:border-charcoal-550 hover:bg-charcoal-600 focus-custom"
+ aria-label="Contact us"
+ >
+ <span className="text-center">Contact us</span>
+ </button>
Alternatively, use your to retain shared behaviors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="flex h-10 w-full cursor-pointer items-center justify-center rounded border border-charcoal-600 bg-tertiary px-8 text-base font-medium transition hover:border-charcoal-550 hover:bg-charcoal-600"> | |
<span className="text-center text-text-bright">Contact us</span> | |
</div> | |
} | |
></Feedback> | |
/> | |
</div> | |
<button | |
type="button" | |
className="flex h-10 w-full items-center justify-center rounded border border-charcoal-600 bg-tertiary px-8 text-base font-medium text-text-bright transition hover:border-charcoal-550 hover:bg-charcoal-600 focus-custom" | |
aria-label="Contact us" | |
> | |
<span className="text-center">Contact us</span> | |
</button> | |
} |
🤖 Prompt for AI Agents
In apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
around lines 816 to 821, the Enterprise CTA is rendered as a styled <div> which
is not keyboard-focusable or accessible; replace it with a native <button
type="button"> (or the existing Button component: <Button
variant="secondary/large" fullWidth className="...">) keeping the same className
and inner text so visual appearance is unchanged, ensure any onClick handler is
attached to the button, and add any necessary aria-label or aria-describedby if
the visual text is insufficient for screen readers.
…-page-improvements
Updates to the pricing pages
Onboarding pricing page
Dashboard pricing page