-
Notifications
You must be signed in to change notification settings - Fork 322
Fix hero section text overlap on small and medium screens #523
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: main
Are you sure you want to change the base?
Fix hero section text overlap on small and medium screens #523
Conversation
|
@skyforge-glitch is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Banner component is updated to address responsive layout issues by conditionally rendering decorative Pattern elements on larger viewports only, while reordering the wrapper's class list for clarity. This prevents horizontal overflow between 640px–1024px viewport widths by hiding these elements on smaller screens. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Banner.jsx (1)
10-11: Pattern visibility changes correctly address the overflow issue.The addition of
hidden lg:blockeffectively resolves the horizontal overflow and text overlap on small and medium screens (640px–1024px) by hiding the decorative patterns below thelgbreakpoint. Themax-w-fullclass provides additional overflow protection on larger screens.Optional: Consider simplifying the positioning classes.
The positioning classes on both Pattern elements are quite complex with multiple breakpoint-specific adjustments. While functionally correct, consider extracting these into named CSS classes or documenting the positioning logic for future maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Banner.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Banner.jsx (2)
src/components/ContainerPattern.jsx (1)
ContainerPattern(10-12)src/components/Pattern.jsx (1)
Pattern(3-59)
🔇 Additional comments (1)
src/components/Banner.jsx (1)
8-8: The positioning change is correct and improves the implementation.The class change from
"overflow-hidden lg:relative"to"relative overflow-hidden"is intentional and functionally sound. The Pattern elements are now hidden on smaller screens using the semantichidden lg:blockclass, making the wrapper's unconditionalrelativepositioning safe — it won't affect layout on sm/md screens where the patterns are hidden, and it provides the necessary positioning context on lg+ screens where they're visible.
|
Hi mentors 👋 This PR addresses the hero section overlap issue on sm–md screens (640–1024px) by adjusting pattern visibility at responsive breakpoints. I noticed PR #516 works on a similar area using layout adjustments. Happy to iterate, rework, or combine approaches based on your feedback. |
Fixes #522
Summary
This PR fixes an issue where the decorative hero pattern overlaps the text on small and medium screen sizes, affecting readability.
The pattern is hidden below the
lgbreakpoint to prevent overlap and layout issues, while preserving the existing desktop layout.Details
smandmdbreakpointsNotes
A future enhancement could explore repositioning the pattern on tablet-sized screens instead of hiding it.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.