Add landing, contact, and trial pages with client-side routing#29
Add landing, contact, and trial pages with client-side routing#29HusseinBaraja merged 26 commits intomainfrom
Conversation
- Load phone and email from SITE_ env vars - Update contact copy and location details - Extend Vite env loading and test coverage
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 52 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughA new client-side routing system was implemented using Hono, introducing Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant RouteView as RouteView Component
participant TrieRouter
participant LinkComponent as Link Component
participant PageComponent as Page Components<br/>(Landing/Contact/Trial)
User->>Browser: Navigate to path (e.g., /contact)
Browser->>RouteView: URL changes (popstate event)
RouteView->>RouteView: useLocation() reads new path
RouteView->>TrieRouter: match('GET', path)
TrieRouter-->>RouteView: returns matched component
RouteView->>PageComponent: render matched page
PageComponent-->>Browser: display page content
User->>LinkComponent: click Link (href="/trial")
LinkComponent->>LinkComponent: intercept click event
alt Internal route
LinkComponent->>Browser: history.pushState(path)
LinkComponent->>RouteView: navigate callback
RouteView->>RouteView: update context path
RouteView->>TrieRouter: match('GET', /trial)
TrieRouter-->>RouteView: returns TrialPage component
RouteView->>PageComponent: render TrialPage
PageComponent-->>Browser: smooth scroll or animate
else External link / hash
LinkComponent->>Browser: allow default navigation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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/web/src/components/layout/Layout.tsx`:
- Around line 18-76: The effect in useEffect that manipulates sectionLinksRef
via gsap.set/gsap.to (and uses isFirstRender and isLandingPage) must clean up
animations on unmount to avoid touching stale DOM; wrap all gsap calls in a
gsap.context tied to the component (or store returned tweens/timelines) and
return a cleanup that calls context.revert() or tween.kill()/timeline.kill() to
stop and remove animations targeting sectionLinksRef.current and links when the
component unmounts.
In `@apps/web/src/components/router/HonoRouter.tsx`:
- Around line 31-37: Handle hash-only links by both scrolling and updating
history: when href startsWith('#' (in the click handler that currently gets the
element and calls el.scrollIntoView)), after scrolling call navigate with the
current pathname plus the hash (e.g., `${window.location.pathname}${href}`) or
use history.pushState to push the new hash so the URL and history are updated;
update the same logic in both places referenced (the block around
document.getElementById/scrollIntoView and the similar block at lines 61-67) so
deep links and back-button work correctly while preserving the scrolling
behavior.
- Around line 18-45: The Link component currently only accepts href, className,
and onClick and incorrectly routes non-HTTP schemes; update Link to accept and
forward all anchor props by changing the signature to capture ...props from
React.AnchorHTMLAttributes<HTMLAnchorElement> and spread them onto the rendered
<a>, and update handleClick so it only hijacks same-origin navigable document
links (e.g., relative paths starting with '/' or same-origin URLs) — detect
external/other schemes (mailto:, tel:, download, target="_blank", different
origin) and let the browser handle them instead of calling navigate(href);
reference Link, handleClick, useLocation, and navigate when making these
changes.
In `@apps/web/src/components/sections/PricingSection.tsx`:
- Around line 143-145: The CTA Link in PricingSection.tsx is rendering as an
inline anchor so the w-full isn't applied; update the Link element (the one with
className containing "w-full text-center bg-surface ...") to be block-level by
adding a utility like "block" (or "inline-block" if preferred) to its className
so it fills the container as intended.
In `@apps/web/src/pages/ContactPage.tsx`:
- Around line 37-42: The handleSubmit function currently logs formData to the
console; remove the debug console.log('Dummy Contact Form Submit: ', formData)
from handleSubmit in ContactPage (or replace it with a production-safe logger if
needed) so no debug output remains before deployment, and keep the rest of the
logic (alert and setFormData) intact.
- Around line 10-11: The current fallbacks for contactPhoneNumber and
contactEmailAddress use literal placeholders ('[PHONE_NUMBER]',
'[EMAIL_ADDRESS]') which will render to users; update ContactPage to either
provide user-friendly fallbacks (e.g., "Not available" or "Contact support") or,
preferably, conditionally render the phone/email sections only when
import.meta.env.SITE_CONTACT_PHONE_NUMBER and
import.meta.env.SITE_CONTACT_EMAIL_ADDRESS are present. Locate the
contactPhoneNumber and contactEmailAddress variables and change the
assignment/usage so the UI shows a non-placeholder message or hides the contact
elements when values are missing (adjust rendering in the ContactPage component
to check these variables before output).
In `@apps/web/src/pages/TrialPage.tsx`:
- Around line 160-167: The inline CSS in TrialPage.tsx using
dangerouslySetInnerHTML (the animate-shine / `@keyframes` shine block) should be
removed and the animation moved into Tailwind config: add a keyframes.shine
entry with 100% left:125% and an animation.shine mapping to "shine 1s
ease-in-out" in tailwind.config.js, then replace usages of the animate-shine
class in the TrialPage component with the Tailwind animation class; this
eliminates the inline style and the static-analysis warning while keeping
behavior identical.
- Around line 43-47: The handleSubmit function contains a debug console.log of
formData that must be removed before production; update the handleSubmit handler
to delete the console.log('Dummy Trial Form Submit: ', formData) call and
instead send formData to your backend (e.g., via fetch/axios to your trial
endpoint) with proper error handling and user feedback, ensuring you reference
the handleSubmit function and the formData variable and handle success/failure
states (disable button/Show loader, catch errors, and surface a user-friendly
message).
- Around line 79-123: The inputs in the form handled by handleSubmit (using
formData and setFormData) lack associated <label> elements which hurts
accessibility; add unique id attributes (e.g., name, phone, company) to each
input and add corresponding <label htmlFor="..."> elements with descriptive text
(Arabic labels matching placeholders), placing them before each input (or use
visually-hidden utility classes like "sr-only" if you want them hidden
visually), and keep existing placeholder/required behavior and styles — update
the JSX around the inputs and ensure ArrowLeft/button markup remains unchanged.
In `@apps/web/vite.config.d.ts`:
- Line 1: The declaration for _default incorrectly intersects multiple Vite
config shapes (import("vite").UserConfig & Promise<...> & UserConfigFnObject &
UserConfigFnPromise & UserConfigFn) when the export should be one of those
forms; replace the intersection with Vite's union alias
import("vite").UserConfigExport so that the exported type reflects a single
valid config export form (update the const _default declaration to use
UserConfigExport and keep the existing exported symbol name).
In `@apps/web/vite.config.js`:
- Around line 2-16: Current vite.config.js shadows vite.config.ts causing
envDir/envPrefix (and thus SITE_* vars) to be omitted; fix by either removing
this vite.config.js file or merging the env settings from the TS config into it:
update the exported defineConfig call in this file (plugins, server, test) to
also include envDir and envPrefix (so SITE_* vars are exposed) and ensure the
same configuration present in the TS version is preserved; reference the
exported defineConfig(...) in vite.config.js when applying the change.
In `@apps/web/vite.config.ts`:
- Around line 18-22: The test block is being type-ignored; import and use
defineConfig from "vitest/config" (instead of the current import from "vite")
and wrap the exported configuration with defineConfig so the test: {
environment: 'jsdom', include: [...] } block is properly typed; remove the //
`@ts-ignore` and ensure the existing test block in vite.config.ts is retained
inside the defineConfig() call to match the pattern used in vitest.config.ts and
convex/vitest.config.ts.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 55215018-1d17-48ee-94c6-310362ee0e93
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.env.exampleapps/web/package.jsonapps/web/src/App.tsxapps/web/src/components/icons.tsapps/web/src/components/layout/Layout.tsxapps/web/src/components/layout/Layout.vitest.tsxapps/web/src/components/router/HonoRouter.tsxapps/web/src/components/sections/CTASection.tsxapps/web/src/components/sections/HeroSection.tsxapps/web/src/components/sections/PricingSection.tsxapps/web/src/index.cssapps/web/src/pages/ContactPage.tsxapps/web/src/pages/ContactPage.vitest.tsxapps/web/src/pages/LandingPage.tsxapps/web/src/pages/TrialPage.tsxapps/web/src/pages/TrialPage.vitest.tsxapps/web/src/test/setupGsapMocks.tsapps/web/vite.config.d.tsapps/web/vite.config.jsapps/web/vite.config.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use the
@cs/*path aliases fromtsconfig.base.jsonfor cross-package imports
Files:
apps/web/src/components/icons.tsapps/web/src/components/sections/HeroSection.tsxapps/web/src/components/sections/PricingSection.tsxapps/web/src/components/sections/CTASection.tsxapps/web/src/pages/LandingPage.tsxapps/web/src/pages/TrialPage.vitest.tsxapps/web/vite.config.jsapps/web/vite.config.d.tsapps/web/src/App.tsxapps/web/src/pages/ContactPage.vitest.tsxapps/web/vite.config.tsapps/web/src/pages/TrialPage.tsxapps/web/src/components/layout/Layout.vitest.tsxapps/web/src/test/setupGsapMocks.tsapps/web/src/pages/ContactPage.tsxapps/web/src/components/layout/Layout.tsxapps/web/src/components/router/HonoRouter.tsx
🧠 Learnings (2)
📚 Learning: 2026-04-01T13:22:06.563Z
Learnt from: CR
Repo: HusseinBaraja/CS PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-01T13:22:06.563Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use the `cs/*` path aliases from `tsconfig.base.json` for cross-package imports
Applied to files:
apps/web/vite.config.ts
📚 Learning: 2026-04-01T13:22:06.563Z
Learnt from: CR
Repo: HusseinBaraja/CS PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-01T13:22:06.563Z
Learning: Applies to packages/config/src/index.ts : Do not make seed-only env vars globally required in `packages/config/src/index.ts`. Keep variables like `SEED_OWNER_PHONE` optional in shared config and enforce them at the CLI command boundary with `requireEnv`
Applied to files:
apps/web/vite.config.ts
🪛 ast-grep (0.42.0)
apps/web/src/pages/TrialPage.tsx
[warning] 159-159: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 27-27: [UnorderedKey] The SITE_CONTACT_EMAIL_ADDRESS key should go before the SITE_CONTACT_PHONE_NUMBER key
(UnorderedKey)
🔇 Additional comments (12)
.env.example (1)
26-27: No changes needed –SITE_prefix is already enabled in vite.config.ts.The
envPrefixinvite.config.tsalready includes'SITE_', so these variables are correctly exposed to the client. The code inContactPage.tsxreads them asimport.meta.env.SITE_CONTACT_PHONE_NUMBERandimport.meta.env.SITE_CONTACT_EMAIL_ADDRESS, which matches the definitions in.env.example. The suggested rename toVITE_SITE_CONTACT_*would break the application.> Likely an incorrect or invalid review comment.apps/web/src/pages/LandingPage.tsx (1)
9-20: Clean page composition and route-ready export.This is a clear and maintainable page aggregator for the landing route.
apps/web/src/pages/TrialPage.vitest.tsx (1)
18-38: Good behavior-focused test for the trial flow.The test validates render, interaction, and submit confirmation in one coherent scenario.
apps/web/src/App.tsx (1)
17-27: Route wiring is clean and consistent with the new page architecture.The route map and provider/view composition are clear and easy to extend.
apps/web/src/pages/ContactPage.vitest.tsx (1)
20-52: Solid coverage of env-backed rendering and form-reset behavior.This test verifies both static content and the full submit/reset interaction path.
apps/web/src/test/setupGsapMocks.ts (1)
17-57:timeline.fromTomock extension is correctly chained.The added type + mock implementation matches the existing timeline API shape used by tests.
apps/web/src/pages/TrialPage.tsx (1)
12-41: GSAP animation at line 16 executes outside the timeline but relies on scope.The
gsap.to('.trial-ambient-blob', ...)call at line 16 runs immediately whenuseGSAPcallback executes. Whilescope: containeris passed touseGSAP, the GSAP library's scoped selector resolution applies to calls made through the context'scontextSafewrapper or within the scoped environment. The directgsap.to()call here should work correctly becauseuseGSAPsets up the scope context before invoking the callback.The animation logic and timeline sequencing look correct.
apps/web/src/components/layout/Layout.vitest.tsx (2)
13-24: Good mock setup for router-aware testing.The mutable
mockPathvariable with reset inafterEachis a reasonable pattern for controlling route state in tests. TheLinkmock correctly passes through essential props.
97-139: Tests properly verify section links behavior across routes.The tests cover:
- Section links present on landing page
- Section links present (but animated out) on non-landing pages
- Pointer-events disabled on non-landing pages
Good coverage of the GSAP animation state management.
apps/web/src/components/layout/Layout.tsx (2)
82-95: Logo link correctly handles navigation vs scroll-to-top.When on the landing page, clicking the logo scrolls to top instead of navigating. When on other pages, the
Linknavigates to/. This is good UX.
97-104: Hash navigation is properly implemented and working correctly.The
Linkcomponent in HonoRouter delegates hash URLs to thenavigatefunction, which extracts the hash and scrolls to the target element with smooth behavior. The implementation includes robust retry logic (10 attempts with 50ms intervals) usingrequestAnimationFrameto handle asynchronous rendering, ensuring the scroll executes after React has flushed updates.apps/web/src/pages/ContactPage.tsx (1)
144-153: Addrequiredattribute to message textarea for consistency with other form fields.The message textarea (line 151) lacks the
requiredattribute while name, phone, and company fields all have it. Since the form's handleSubmit contains no validation logic and relies on HTML5 form validation, the message field should either haverequiredfor consistency or have a comment explaining why it's optional.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
New Features
Tests
Chores