Redesign enterprise page#670
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Enterprise page fetches /private/plans, derives the Enterprise/payg plan and locale-aware URLs, updates LD+JSON offers, adds new enterprise copy keys, and replaces static markup with data-driven arrays that render hero, customers, scale levers, approval tracks, decision criteria, pillars, feature groups, stats, and a final CTA. ChangesEnterprise Page Redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/enterprise.astro (1)
35-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t publish a
$0enterprise offer in structured data.If the plan lookup fails or
price_mis missing, the current fallback makes the enterprise plan look free to search engines while the UI says “Custom”. Omit theoffersblock when no concrete price is available, or model it explicitly as quote-based pricing instead.Suggested fix
- offers: [ - { - price: payg?.price_m?.toString() || '0', - priceCurrency: 'USD', - availability: 'https://schema.org/InStock', - priceValidUntil: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString().split('T')[0], - }, - ], + ...(payg?.price_m != null + ? { + offers: [ + { + price: payg.price_m.toString(), + priceCurrency: 'USD', + availability: 'https://schema.org/InStock', + priceValidUntil: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString().split('T')[0], + }, + ], + } + : {}),🤖 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 `@apps/web/src/pages/enterprise.astro` around lines 35 - 40, The structured-data `offers` block is publishing a $0 price when `payg?.price_m` is absent because of the fallback `price: payg?.price_m?.toString() || '0'`; update the logic that builds `offers` (the offers array and the `price: payg?.price_m?.toString()` expression) to omit the entire `offers` entry when `payg.price_m` is null/undefined/non-numeric, or alternatively render a non-price quote-based model (e.g., set no `offers` or include a descriptive `priceSpecification` indicating “By quote”); ensure the conditional uses the `payg` variable and `price_m` field to gate inclusion so search engines never see a default "0" USD price.
🤖 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.
Inline comments:
In `@apps/web/src/pages/enterprise.astro`:
- Line 244: The image alt text currently uses alt={customer.name} which names
the company but isn't descriptive of the image; update the img element that
references customer.logo and customer.name to provide descriptive alt text such
as the company name plus "logo" (e.g., `${customer.name} logo`) so screen
readers get clear context while leaving src={customer.logo} unchanged.
- Around line 124-162: The hard-coded English strings in the page data arrays
(pillars and includedGroups) cause mixed-language UI; add corresponding keys to
the locale message catalog and replace literals with locale lookups (use the
existing m.* pattern with the { locale } option). For example, create message
keys for the three pillar points currently hard-coded (e.g.
solutions_enterprise_sol3_point2/3) and for every item in includedGroups, then
update the pillars array and includedGroups array to call m.<key>({}, { locale
}) instead of literal strings; also apply the same replacements referenced in
the comment for the similar block around lines 198-210.
- Around line 12-13: The code assumes response.json() yields an array and
directly uses plansAll.find to set payg, which can throw during SSR; wrap the
fetch/parse in a try/catch, verify response.ok and that the parsed value is an
array (Array.isArray(parsed)), and if not, set plansAll to an empty array (or a
safe default) before calling find so payg is undefined instead of crashing;
update the logic around plansAll and payg to use the validated value (and log or
handle errors) so the page can degrade gracefully when the payload is invalid.
---
Outside diff comments:
In `@apps/web/src/pages/enterprise.astro`:
- Around line 35-40: The structured-data `offers` block is publishing a $0 price
when `payg?.price_m` is absent because of the fallback `price:
payg?.price_m?.toString() || '0'`; update the logic that builds `offers` (the
offers array and the `price: payg?.price_m?.toString()` expression) to omit the
entire `offers` entry when `payg.price_m` is null/undefined/non-numeric, or
alternatively render a non-price quote-based model (e.g., set no `offers` or
include a descriptive `priceSpecification` indicating “By quote”); ensure the
conditional uses the `payg` variable and `price_m` field to gate inclusion so
search engines never see a default "0" USD price.
🪄 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: 5516cf5f-a316-4497-9d76-97db56d41b92
📒 Files selected for processing (1)
apps/web/src/pages/enterprise.astro
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@apps/web/src/pages/enterprise.astro`:
- Line 254: Replace the hardcoded CTA text "View pricing" with the i18n message
call m.view_pricing({}, { locale }) so the button uses the current locale;
locate the literal "View pricing" in apps/web/src/pages/enterprise.astro (the
CTA element) and swap it to call m.view_pricing, ensuring the surrounding scope
provides the locale variable (or pass the page's locale prop into the component)
and import/ensure the m message bundle is available in this file.
🪄 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: 7a7e1818-8f23-4164-b7f5-9a430bf9722e
📒 Files selected for processing (1)
apps/web/src/pages/enterprise.astro
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/pages/enterprise.astro (1)
148-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the remaining hero metric copy.
"6 M4 builds"is still hard-coded English, so translated enterprise routes will render mixed-language copy in this card. Please move this label intom.*(or compose the numeric value with a localized noun) and apply the same treatment to any other visible literals in this block.🤖 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 `@apps/web/src/pages/enterprise.astro` around lines 148 - 155, The card metric "6 M4 builds" is hard-coded; replace it with a localization key (e.g., add/use m.enterprise_native_builds_metric or compose a number + localized noun via m.*) and call it with the same locale parameter as the existing m.enterprise_native_builds_title and m.enterprise_native_builds_desc so translated routes don't mix languages; also audit this card block (the object with title/metric/description alongside m.enterprise_compliance_procurement_title/description) and move any other visible literals into the m.* localization namespace with the locale argument.
🤖 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.
Inline comments:
In `@apps/web/src/pages/enterprise.astro`:
- Around line 13-23: The fetch to /private/plans can hang; update the block that
fetches plans (the try/await fetch using plansAll) to use an AbortController
with a configurable timeout (e.g., const timeoutMs) so the request is aborted if
it exceeds the timeout, clear the timer on success, and handle the abort/error
in the existing catch to set plansAll = []; specifically modify the fetch call
to pass { signal: controller.signal }, create a setTimeout that calls
controller.abort(), and ensure you clearTimeout when response.ok and after
parsing to avoid leaks.
---
Duplicate comments:
In `@apps/web/src/pages/enterprise.astro`:
- Around line 148-155: The card metric "6 M4 builds" is hard-coded; replace it
with a localization key (e.g., add/use m.enterprise_native_builds_metric or
compose a number + localized noun via m.*) and call it with the same locale
parameter as the existing m.enterprise_native_builds_title and
m.enterprise_native_builds_desc so translated routes don't mix languages; also
audit this card block (the object with title/metric/description alongside
m.enterprise_compliance_procurement_title/description) and move any other
visible literals into the m.* localization namespace with the locale argument.
🪄 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: 99aede42-96dd-4787-9455-b59594f3febf
📒 Files selected for processing (2)
apps/shared/copy/messages.tsapps/web/src/pages/enterprise.astro
✅ Files skipped from review due to trivial changes (1)
- apps/shared/copy/messages.ts
|
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
1 participant
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
Validation
Summary by CodeRabbit