-
-
Notifications
You must be signed in to change notification settings - Fork 0
Fix update homepage #326
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
base: master
Are you sure you want to change the base?
Fix update homepage #326
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR enhances the homepage by adding new “Tonightpass,” “Services,” and “Reviews” sections, and refines the existing featured section’s decorations and styling for better responsiveness.
- Introduces three new visitor landing components and integrates them into the landing page.
- Updates the Featured section with decorative SVG elements and adjusts overflow/z-index.
- Tweaks project-wide styling and editor settings (layout import, VSCode settings).
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tailwind.config.ts | Added trailing whitespace (no functional change) |
| src/screens/marketing/landing/visitor/index.tsx | Imported and rendered <Tonightpass/>, <Services/>, <Reviews/> |
| src/components/marketing/landing/visitor/tonightpass/index.tsx | New Tonightpass section with CTA button and image |
| src/components/marketing/landing/visitor/services/index.tsx | New Services section with scroll-driven color changes |
| src/components/marketing/landing/visitor/reviews/index.tsx | New Reviews section with mapped testimonials grid |
| src/components/marketing/landing/visitor/featured/index.tsx | Added decorative SVGs, removed overflow-hidden, adjusted z-index |
| src/components/marketing/landing/testimonial/index.tsx | Created reusable Testimonial component |
| src/app/layout.tsx | Added missing semicolon on Google font import |
| .vscode/settings.json | Added project editor settings |
| const containerRef = useRef<HTMLDivElement | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| setIsMobile(window.innerWidth < 768); |
Copilot
AI
Jul 1, 2025
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.
You only set isMobile once on mount. Consider adding a window resize listener to update isMobile on viewport changes for consistent behavior.
| setIsMobile(window.innerWidth < 768); | |
| const updateIsMobile = () => setIsMobile(window.innerWidth < 768); | |
| updateIsMobile(); |
| const handleScroll = () => { | ||
| if (!containerRef.current || isMobile) return; | ||
|
|
||
| const containerTop = |
Copilot
AI
Jul 1, 2025
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.
Calculating containerTop on every scroll event can be expensive. Consider caching its value outside the handler or throttling/debouncing the scroll listener.
| {services.map((service, index) => ( | ||
| <div key={index} className="min-h-[200px] mb-12"> |
Copilot
AI
Jul 1, 2025
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.
Avoid using the array index as key in a list; use a stable unique identifier (e.g., service.title) to prevent rendering glitches.
| {services.map((service, index) => ( | |
| <div key={index} className="min-h-[200px] mb-12"> | |
| {services.map((service) => ( | |
| <div key={service.title} className="min-h-[200px] mb-12"> |
| </button> | ||
| </Link> | ||
| <div className="w-full max-w-screen-lg mt-8 mb-4 px-4 md:px-0"> | ||
| {" "} |
Copilot
AI
Jul 1, 2025
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.
[nitpick] This explicit {" "} text node for whitespace is unnecessary; consider removing it or handling spacing with CSS margins.
| {" "} |
improved responsive on section featured, tonightpass, services and reviews.