-
Notifications
You must be signed in to change notification settings - Fork 283
Fix horizontal overflow and improve accessibility in Project page layout #473
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 horizontal overflow and improve accessibility in Project page layout #473
Conversation
|
@Saniya1976 is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe projects page was updated to prevent horizontal layout overflow by adding overflow constraints to outer and inner containers. Accessibility improvements were made to the GSOC logo element by adding proper ARIA attributes. 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
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/projects.jsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/projects.jsx (1)
src/components/Container.jsx (2)
Container(30-39)Container(30-39)
| role="img" | ||
| aria-label="GSOC Logo" |
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.
🛠️ Refactor suggestion | 🟠 Major
Apply consistent accessibility pattern to both logo elements.
The addition of role="img" and aria-label improves accessibility for this decorative logo. However, the second logo element (lines 112-116) has aria-label="Logo" but lacks the role="img" attribute, creating an inconsistency.
Apply this diff to ensure both logos follow the same accessibility pattern:
<div
className="hidden md:block w-[75px] h-[75px] m-2 bg-cover bg-center absolute right-10"
style={{ backgroundImage: "url('/logo.png')" }}
+ role="img"
aria-label="Logo"
></div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/projects.jsx around lines 104-116, the two logo elements use
inconsistent accessibility attributes: the first has role="img" and aria-label
but the second only has aria-label="Logo"; update the second logo element (lines
~112-116) to also include role="img" so both logos follow the same accessibility
pattern, keeping the existing aria-label value.
| <Container className="mt-20 mb-28" sx={{ overflowX: 'hidden' }}> | ||
| <Container.Inner sx={{ overflowX: 'hidden', maxWidth: '100%' }}> |
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.
🧩 Analysis chain
Verify Container.Inner exists and consider fixing the root cause of overflow.
Two concerns:
-
Container.Inner may not accept
sxprops: The provided Container component definition shows that props are spread toOuterContainer, butContainer.Inneris not shown in the relevant code snippets. Please verify thatContainer.Innerexists as a separate exported component and properly acceptssxprops. -
Hiding overflow masks the problem: Using
overflowX: 'hidden'treats the symptom rather than the cause. The actual overflow likely stems from the banner wrapper's negative margins (line 125:marginLeft: 'calc(-50vw + 50%)') or elements exceeding viewport constraints. This approach may clip content rather than properly constraining the layout.
Consider investigating and fixing the root cause instead.
Run this script to verify the Container component's structure:
🏁 Script executed:
#!/bin/bash
# Verify Container.Inner exists and check its implementation
# Find the Container component definition
echo "=== Container component definition ==="
ast-grep --pattern $'export const Container = $$$'
echo -e "\n=== Check for Container.Inner export ==="
rg -n -A5 'Container\.Inner\s*=' src/components/Container.jsx
echo -e "\n=== Check for InnerContainer definition ==="
rg -n -A10 'InnerContainer\s*=' src/components/Container.jsxLength of output: 1042
I need to verify one more critical detail: whether OuterContainer is an MUI-compatible component that can handle sx props.
Let me verify the OuterContainer definition and check the actual usage context in projects.jsx:Now I need to verify the OuterContainer definition and check the projects.jsx context:
🏁 Script executed:
#!/bin/bash
# Check OuterContainer definition and projects.jsx context
echo "=== OuterContainer definition in Container.jsx ==="
rg -n -A5 'const OuterContainer' src/components/Container.jsx
echo -e "\n=== Check for styled import in Container.jsx ==="
head -30 src/components/Container.jsx | grep -E "import.*styled|from.*emotion|from.*@mui"
echo -e "\n=== View projects.jsx lines 120-145 (banner and overflow context) ==="
sed -n '120,145p' src/pages/projects.jsxLength of output: 1313
sx prop won't work on plain HTML divs—Container and Container.Inner are not MUI components.
The sx props at lines 138-139 will not apply any styles. Both Container and Container.Inner render plain <div> elements (not MUI System components like Box). The MUI sx prop only works on MUI components or styled components; it's silently ignored on plain HTML elements.
Instead:
- Replace with
style={{ overflowX: 'hidden', maxWidth: '100%' }}, or - Wrap in an MUI
Boxcomponent, or - Use Tailwind classes like
className="overflow-hidden"
Additionally, the bannerWrapper style object (line 124) already sets overflow: 'hidden', so verify whether this new constraint actually addresses the intended layout issue.
🤖 Prompt for AI Agents
In src/pages/projects.jsx around lines 138-139, the sx props on Container and
Container.Inner are ineffective because these are plain divs not MUI System
components; replace the sx usage with one of the following fixes: (1) use inline
style={{ overflowX: 'hidden', maxWidth: '100%' }} on those divs, (2) wrap the
content in an MUI Box (or another MUI component) and move sx there, or (3) apply
equivalent Tailwind classes like className="overflow-hidden max-w-full"; also
check the bannerWrapper style at line 124 which already sets overflow: 'hidden'
and remove redundant constraints if they’re unnecessary.
This pull request addresses the horizontal overflow issue on the Project page that was causing an unwanted horizontal scroll bar in desktop Chrome browsers. Additionally, an accessibility improvement was made for the GSOC Logo.
Changes include:
Added sx={{ overflowX: 'hidden' }} to the Container component to prevent horizontal overflow. (Line 149)
Added sx={{ overflowX: 'hidden', maxWidth: '100%' }} to Container.Inner to restrict width within viewport. (Line 150)
Improved accessibility by replacing alt="GSOC Logo" on the image with role="img" and aria-label="GSOC Logo" attributes. (Line 111)
These changes ensure a smoother UI experience free from layout shifts and enhance the accessibility compliance of the logo image.
Issue: Resolves #473