Skip to content

Conversation

@Saniya1976
Copy link

@Saniya1976 Saniya1976 commented Oct 29, 2025

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

@vercel
Copy link

vercel bot commented Oct 29, 2025

@Saniya1976 is attempting to deploy a commit to the AOSSIE Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link

coderabbitai bot commented Oct 29, 2025

Walkthrough

The 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

Cohort / File(s) Summary
Layout overflow fixes
src/pages/projects.jsx
Added overflowX: 'hidden' to outer container and overflowX: 'hidden', maxWidth: '100%' to inner container to prevent horizontal scroll. Updated GSOC logo with role="img" and aria-label="GSOC Logo" for accessibility.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file with straightforward CSS property additions
  • Accessibility attributes follow standard patterns
  • No logic or control flow changes
  • Direct resolution of horizontal overflow issue

Poem

🐰 The page once wandered left and right,
With scrollbars stealing desktop sight,
But now constraints keep overflow tame,
And ARIA labels know the logo's name!
A tidy fix, both wise and light.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Fix horizontal overflow and improve accessibility in Project page layout" accurately and directly addresses both main changes in the changeset. It clearly summarizes the horizontal overflow fixes (overflowX styling on containers) and the accessibility improvement to the GSOC logo (role and aria-label attributes). The title is concise, specific, and would allow teammates scanning history to quickly understand the primary changes made to the project page.
Linked Issues Check ✅ Passed The PR successfully addresses all coding requirements from linked issue #471. The horizontal overflow fix is implemented through the addition of sx={{ overflowX: 'hidden' }} to the Container component and sx={{ overflowX: 'hidden', maxWidth: '100%' }} to Container.Inner, which directly constrains container widths within the viewport and eliminates the horizontal scrollbar issue. Additionally, the accessibility improvement to the GSOC logo component is explicitly stated as an objective in the PR description and enhances the overall quality of the changes related to the project page layout.
Out of Scope Changes Check ✅ Passed All code changes in the PR are related to improving the Project page layout and are appropriately scoped to address the identified issues. The horizontal overflow fixes directly resolve the requirements specified in issue #471 by constraining container widths and preventing layout shifts. The accessibility improvement to the GSOC logo, while going slightly beyond the explicit scope of issue #471, is related to the same component and page and is explicitly documented in the PR objectives and title, making it an intentional and justified enhancement rather than unintended scope creep.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between dfb8668 and 7268c4f.

📒 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)

Comment on lines +104 to +105
role="img"
aria-label="GSOC Logo"
Copy link

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.

Comment on lines +138 to +139
<Container className="mt-20 mb-28" sx={{ overflowX: 'hidden' }}>
<Container.Inner sx={{ overflowX: 'hidden', maxWidth: '100%' }}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify Container.Inner exists and consider fixing the root cause of overflow.

Two concerns:

  1. Container.Inner may not accept sx props: The provided Container component definition shows that props are spread to OuterContainer, but Container.Inner is not shown in the relevant code snippets. Please verify that Container.Inner exists as a separate exported component and properly accepts sx props.

  2. 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.jsx

Length 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.jsx

Length 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 Box component, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant