Remove duplicate media query for timeline lines#562
Remove duplicate media query for timeline lines#562prashantkumarniraj wants to merge 4 commits intoAOSSIE-Org:mainfrom
Conversation
|
@prashantkumarniraj is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughUpdated social media icons across components from faTwitter to faXTwitter, restructured contact and social links between Link and anchor elements in Footer and home pages, and removed responsive breakpoint classes from About page headings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (1 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: 1
🤖 Fix all issues with AI agents
In `@src/pages/about.jsx`:
- Around line 146-148: The h1 currently uses non-monotonic Tailwind sizes
("text-6xl md:text-5xl lg:text-6xl") causing the heading to be largest on
mobile; update the className on the H1 element (the ABOUT heading) to use a
monotonic scale such as "text-4xl md:text-5xl lg:text-6xl" or add an sm
breakpoint (e.g., "text-5xl sm:text-6xl md:text-5xl lg:text-6xl") depending on
the intended design so mobile is not the largest—locate the H1 with className
containing text-6xl and adjust the size tokens accordingly.
🧹 Nitpick comments (3)
src/pages/about.jsx (1)
182-184: GSOC heading responsive classes look reasonable, butlg:text-6xlis redundant.The progression
text-2xl sm:text-6xl lg:text-6xlis sensible, but sincesm:text-6xlalready sets6xlfrom thesmbreakpoint onward, thelg:text-6xlclass is a no-op. You can safely remove it unless you anticipate inserting an intermediatemd:size later.src/pages/index.jsx (1)
90-96: Update aria-label and href to reflect the X rebrand.The icon was updated to
faXTwitter, but the aria-label still reads"Follow on Twitter"(line 91) and the href still points totwitter.com(line 93). Consider updating to"Follow on X"(or"Follow on X (Twitter)") andhttps://x.com/aossie_orgfor consistency with the icon change.Suggested diff
<Link - aria-label="Follow on Twitter" + aria-label="Follow on X" className="text-zinc-500 transition hover:text-[`#00843D`] dark:text-zinc-400 dark:hover:text-yellow-400" - href="https://twitter.com/aossie_org" + href="https://x.com/aossie_org" > <FontAwesomeIcon icon={faXTwitter} size="2xl" /> </Link>src/components/Footer.jsx (1)
64-70: Same asindex.jsx: update aria-label and href for the X rebrand.The icon is
faXTwitterbut the aria-label says"Follow on Twitter"and the href points totwitter.com. For consistency with the icon change, consider updating both.Suggested diff
<Link - aria-label="Follow on Twitter" + aria-label="Follow on X" className="text-zinc-400 hover:text-[`#00843D`] dark:text-zinc-400 dark:hover:text-yellow-400 transition" - href="https://twitter.com/aossie_org" + href="https://x.com/aossie_org" > <FontAwesomeIcon icon={faXTwitter} size="xl" /> </Link>
| <h1 style={{ marginBottom: '20px' }} className="font-mono text-6xl font-extrabold tracking-tighter text-[#32a852] dark:text-yellow-400 md:text-5xl lg:text-6xl"> | ||
| ABOUT | ||
| </h1> |
There was a problem hiding this comment.
Non-monotonic responsive font sizing — heading is largest on mobile.
With text-6xl md:text-5xl lg:text-6xl, the base (mobile) size is 6xl, which then shrinks to 5xl at the md breakpoint before returning to 6xl at lg. This is an unusual pattern where the heading is paradoxically largest on the smallest screens. Was the removal of sm:text-6xl intentional here, or should the base size be reduced (e.g., text-4xl or text-5xl) so that mobile renders a smaller heading?
🤖 Prompt for AI Agents
In `@src/pages/about.jsx` around lines 146 - 148, The h1 currently uses
non-monotonic Tailwind sizes ("text-6xl md:text-5xl lg:text-6xl") causing the
heading to be largest on mobile; update the className on the H1 element (the
ABOUT heading) to use a monotonic scale such as "text-4xl md:text-5xl
lg:text-6xl" or add an sm breakpoint (e.g., "text-5xl sm:text-6xl md:text-5xl
lg:text-6xl") depending on the intended design so mobile is not the
largest—locate the H1 with className containing text-6xl and adjust the size
tokens accordingly.
This PR removes a redundant media query for the
.line-leftand.line-rightclasses. The styles were already covered by the existing
max-width: 768pxrule, so the additional
480pxbreakpoint was unnecessary.No visual changes were introduced.
Fixes #460
Summary by CodeRabbit