feat(marketing): simplify product menu and show yearly discount#3691
Conversation
Remove the feature card from the Product nav dropdown; show $20 struck through next to $15 on the Pro pricing card when Yearly is selected to make the discount explicit. Pin the price row height so toggling between Monthly and Yearly doesn't cause a vertical shift.
📝 WalkthroughWalkthroughTwo UI refinements: the Product dropdown navigation is simplified by removing a feature card component and consolidating to a single-column list layout, while the pricing card now displays strikethrough pricing to highlight savings for yearly billing cycles. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR makes two focused marketing-site changes: it simplifies the Product nav dropdown to a plain link list by removing the Confidence Score: 5/5Safe to merge — changes are UI-only with no data, auth, or runtime risk. Both changes are small, well-scoped UI updates. The only finding is a P2 defensive guard for a future-proof edge case that doesn't affect current data. All three price kinds are handled correctly in resolvePrice, and the unused SupersetLogo import is properly removed. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/marketing/src/app/components/Header/components/DesktopNav/DesktopNav.tsx | Removes the FeatureCard component and its SupersetLogo import; simplifies the Product dropdown to a plain link list at w-[320px]. |
| apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx | Adds strikethrough price display and fixes price-row height with h-10/leading-none; resolvePrice extended cleanly for all three price kinds. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PricingCard renders] --> B{tier.price.kind?}
B -->|fixed| C[display=fixed, strikethrough=null]
B -->|custom| D[display=custom, strikethrough=null]
B -->|variable| E{isYearly?}
E -->|true| F[display=yearly.display\nstrikethrough=monthly.display]
E -->|false| G[display=monthly.display\nstrikethrough=null]
C --> H[Render price row]
D --> H
F --> H
G --> H
H --> I{strikethrough set?}
I -->|yes| J[Render struck-through span + current price]
I -->|no| K[Render current price only]
J --> L[Fixed h-10 row — no layout shift]
K --> L
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx
Line: 103
Comment:
**Strikethrough shown even when prices are identical**
`strikethrough` is always set to `tier.price.monthly.display` when `isYearly` is true, regardless of whether there's actually a discount. If a future `variable` tier has the same `monthly.display` and `yearly.display` (e.g. a launch price with no yearly discount), the UI would show a struck-through price identical to the current price, silently misleading users. A simple guard would prevent this:
```suggestion
strikethrough: isYearly && tier.price.monthly.display !== entry.display ? tier.price.monthly.display : null,
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(marketing): simplify product menu a..." | Re-trigger Greptile
| return { display: entry.display, note: entry.note, cadence: entry.cadence }; | ||
| return { | ||
| display: entry.display, | ||
| strikethrough: isYearly ? tier.price.monthly.display : null, |
There was a problem hiding this comment.
Strikethrough shown even when prices are identical
strikethrough is always set to tier.price.monthly.display when isYearly is true, regardless of whether there's actually a discount. If a future variable tier has the same monthly.display and yearly.display (e.g. a launch price with no yearly discount), the UI would show a struck-through price identical to the current price, silently misleading users. A simple guard would prevent this:
| strikethrough: isYearly ? tier.price.monthly.display : null, | |
| strikethrough: isYearly && tier.price.monthly.display !== entry.display ? tier.price.monthly.display : null, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx
Line: 103
Comment:
**Strikethrough shown even when prices are identical**
`strikethrough` is always set to `tier.price.monthly.display` when `isYearly` is true, regardless of whether there's actually a discount. If a future `variable` tier has the same `monthly.display` and `yearly.display` (e.g. a launch price with no yearly discount), the UI would show a struck-through price identical to the current price, silently misleading users. A simple guard would prevent this:
```suggestion
strikethrough: isYearly && tier.price.monthly.display !== entry.display ? tier.price.monthly.display : null,
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx (1)
83-106: Optional: tighten the return type ofresolvePrice.Because the
fixed/custombranches returnnote: ""(string literal) andstrikethrough: null, while the monthly/yearly branch returnsnote: entry.note(likely string) andstrikethrough: string | null, TypeScript will infer a union across branches. An explicit return type would make the contract clearer and catch accidental shape drift between branches.type ResolvedPrice = { display: string; strikethrough: string | null; note: string; cadence: string | null | undefined; }; function resolvePrice(tier: PricingTier, isYearly: boolean): ResolvedPrice { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx` around lines 83 - 106, The resolvePrice function returns inconsistent shapes across branches which causes TypeScript to widen the inferred return type; declare an explicit ResolvedPrice type (e.g., display: string; strikethrough: string | null; note: string; cadence: string | null | undefined) and annotate resolvePrice(tier: PricingTier, isYearly: boolean): ResolvedPrice, then update the fixed/custom branches to return values matching that shape (set cadence to null or undefined and ensure note is a string) so all branches conform to ResolvedPrice and prevent union widening.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx`:
- Around line 41-48: The strikethrough price in PricingCard is only visually
indicated with the "line-through" class so screen readers will read both values
indistinguishably; update the JSX rendering of the strikethrough (the element
that uses the strikethrough variable) to be semantically explicit for assistive
tech—for example replace the plain span with an <s> (or add role="text" and an
aria-label) or prepend a visually-hidden label like "Original price" to the
strikethrough element so screen readers announce it as the old price while
keeping the visual styling; ensure you modify the element that currently renders
{strikethrough} in PricingCard so the displayed value and the accessible label
convey "old/original price" versus the current {display} price.
---
Nitpick comments:
In
`@apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx`:
- Around line 83-106: The resolvePrice function returns inconsistent shapes
across branches which causes TypeScript to widen the inferred return type;
declare an explicit ResolvedPrice type (e.g., display: string; strikethrough:
string | null; note: string; cadence: string | null | undefined) and annotate
resolvePrice(tier: PricingTier, isYearly: boolean): ResolvedPrice, then update
the fixed/custom branches to return values matching that shape (set cadence to
null or undefined and ensure note is a string) so all branches conform to
ResolvedPrice and prevent union widening.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6297dc10-c121-43f8-a450-7477de11ec5c
📒 Files selected for processing (2)
apps/marketing/src/app/components/Header/components/DesktopNav/DesktopNav.tsxapps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx
| {strikethrough && ( | ||
| <span className="text-2xl font-medium tracking-tight leading-none text-muted-foreground line-through"> | ||
| {strikethrough} | ||
| </span> | ||
| )} | ||
| <span className="text-4xl font-medium tracking-tight leading-none text-foreground"> | ||
| {display} | ||
| </span> |
There was a problem hiding this comment.
Consider accessibility for the strikethrough price.
The line-through class is purely visual — screen readers won't convey that $20 is the old/struck-through price, so users will hear "$20 $15 per user/month" which is confusing. Consider using a semantic <s> element and/or an aria-label/visually-hidden prefix to communicate the discount.
🛡️ Suggested change
- {strikethrough && (
- <span className="text-2xl font-medium tracking-tight leading-none text-muted-foreground line-through">
- {strikethrough}
- </span>
- )}
+ {strikethrough && (
+ <s
+ aria-label={`Was ${strikethrough}`}
+ className="text-2xl font-medium tracking-tight leading-none text-muted-foreground"
+ >
+ {strikethrough}
+ </s>
+ )}📝 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.
| {strikethrough && ( | |
| <span className="text-2xl font-medium tracking-tight leading-none text-muted-foreground line-through"> | |
| {strikethrough} | |
| </span> | |
| )} | |
| <span className="text-4xl font-medium tracking-tight leading-none text-foreground"> | |
| {display} | |
| </span> | |
| {strikethrough && ( | |
| <s | |
| aria-label={`Was ${strikethrough}`} | |
| className="text-2xl font-medium tracking-tight leading-none text-muted-foreground" | |
| > | |
| {strikethrough} | |
| </s> | |
| )} | |
| <span className="text-4xl font-medium tracking-tight leading-none text-foreground"> | |
| {display} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/marketing/src/app/pricing/components/PricingTiers/components/PricingCard/PricingCard.tsx`
around lines 41 - 48, The strikethrough price in PricingCard is only visually
indicated with the "line-through" class so screen readers will read both values
indistinguishably; update the JSX rendering of the strikethrough (the element
that uses the strikethrough variable) to be semantically explicit for assistive
tech—for example replace the plain span with an <s> (or add role="text" and an
aria-label) or prepend a visually-hidden label like "Original price" to the
strikethrough element so screen readers announce it as the old price while
keeping the visual styling; ensure you modify the element that currently renders
{strikethrough} in PricingCard so the displayed value and the accessible label
convey "old/original price" versus the current {display} price.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Summary
$20struck through next to$15on the Pro pricing card when Yearly is selected, making the 25% discount explicit.leading-noneso toggling Monthly/Yearly doesn't cause a vertical shift.Test plan
$20$15 per user/month" with$20at the left edge, no height change on the card or billing toggle.Summary by cubic
Simplifies the Product dropdown to two links and makes the Pro yearly discount explicit. Locks the price row height so switching Monthly/Yearly doesn't shift the card.
Written for commit 58d22a8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Style