This PR fixes issue #1286 where mobile screens showed extra blank space#1289
This PR fixes issue #1286 where mobile screens showed extra blank space#1289iaadillatif wants to merge 1 commit intorecodehive:mainfrom
Conversation
… blank space above the homepage hero content.
|
@iaadillatif is attempting to deploy a commit to the recode Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. The estimated time for response is 5–8 hrs. In the meantime, please provide all necessary screenshots and make sure you run - npm build run , command and provide a screenshot, a video recording, or an image of the update you made below, which helps speed up the review and assignment. If you have questions, reach out to LinkedIn. Your contributions are highly appreciated!😊 Note: I maintain the repo issue every day twice at 8:00 AM IST and 9:00 PM IST. If your PR goes stale for more than one day, you can tag and comment on this same issue by tagging @sanjay-kv. We are here to help you on this journey of open source. Consistent 20 contributions are eligible for sponsorship 💰 🎁 check our list of amazing people we sponsored so far: GitHub Sponsorship. ✨ 📚Your perks for contribution to this community 👇🏻
If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
|
✅ Synchronized metadata from Issue #1286:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses the reported extra top whitespace on mobile/tablet viewports by adjusting navbar/announcement-bar behavior, and completes a homepage refactor by moving the hero/header rendering into a dedicated Hero component (including aligning FloatingContributors embed prop naming).
Changes:
- Adjusted mobile/tablet navbar CSS to remove top offset and prevent hidden announcement-bar spacing.
- Replaced homepage
Headerusage with a newHerocomponent + isolatedhero.css. - Renamed
FloatingContributorsembed prop fromheaderEmbeddedtoheroEmbeddedand updated its README.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/index.tsx | Switches homepage to render the new Hero component and adjusts hero wrapper spacing. |
| src/css/custom.css | Adds mobile/tablet navbar/announcement-bar rules intended to eliminate top whitespace. |
| src/components/hero/hero.tsx | Introduces the dedicated Hero component and wires FloatingContributors into it. |
| src/components/hero/hero.css | Renames/updates hero styles to match the new component structure and responsive behavior. |
| src/components/header.tsx | Removes the legacy header implementation. |
| src/components/FloatingContributors/index.tsx | Renames the embed prop to heroEmbedded and updates behavior accordingly. |
| src/components/FloatingContributors/README.md | Updates documentation to use heroEmbedded. |
Comments suppressed due to low confidence (3)
src/components/hero/hero.tsx:57
- In JSX,
<br>is typically written as a self-closing tag (<br />). Using<br></br>is valid but inconsistent with the<br />used a few lines above and can trip formatting/linting rules; consider switching these to<br />for consistency.
src/components/hero/hero.tsx:17 hero.tsximports Framer Motion viaimport * as FramerMotionand then castsmotionout of the module namespace. This drops the normalmotion.*typings (props become effectivelyunknown) and deviates from how Framer Motion is imported everywhere else in the repo (e.g.import { motion } from "framer-motion"). Prefer using the standard named import so TS can type-check the motion components and avoid runtime surprises if module shape changes.
src/pages/index.tsx:45- This change removes the
<Head>injection of theamp-auto-adsscript from the homepage. Other pages in the codebase still inject this script (e.g. showcase, sponsors), so this is a functional change beyond the stated whitespace/hero refactor. Please confirm this is intentional; if the homepage should still load AMP auto ads, keep theHeadblock here or move the script injection to a shared/global location.
<Layout
title={`${siteConfig.title}`}
description="Welcome to recode hive. Learn the basics to advanced concepts of web development. sql, python, Cloud, GitHub and more."
>
{/* ✅ Wrap in solid background to fix light mode */}
<div
className="transition-colors duration-300"
style={{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /* Clean mobile navbar layout - prevent overflow */ | ||
| .navbar { | ||
|
|
There was a problem hiding this comment.
There’s an extra blank line inside the .navbar rule which looks accidental and adds noise to custom.css. Consider removing it to keep the stylesheet tidy.
| className={`floating-contributors-container ${heroEmbedded ? "header-embedded" : ""}`} | ||
| initial={{ | ||
| opacity: 0, | ||
| y: headerEmbedded ? 0 : 50, | ||
| scale: headerEmbedded ? 1 : 0.9, | ||
| y: heroEmbedded ? 0 : 50, | ||
| scale: heroEmbedded ? 1 : 0.9, |
There was a problem hiding this comment.
The prop was renamed to heroEmbedded, but the conditional class name is still "header-embedded". This naming mismatch is confusing for future maintenance and makes it harder to grep/understand which styles apply to the hero vs other contexts. Consider renaming the CSS hook to hero-embedded (and updating the corresponding selectors in FloatingContributors.css), or keep the prop name aligned with the existing header-embedded styling.
This PR resolves the mobile top whitespace issue and finalizes the homepage component separation by migrating hero rendering into a dedicated component.
Fixes #1286
Type of Change
Changes Made
Mobile Navbar / Top Offset Fix
custom.css.Mobile UI before:
Mobile UI after changes:
Hero Rendering Refactor
hero.tsxcomponent.hero.cssfor isolated styling.index.tsx.src/components/header/header.tsx.Floating Contributors Prop Alignment
heroEmbeddedinindex.tsx.README.md.Behavior After Fix
Desktop Hero rendering correctly
Dependencies
Checklist
npm run buildand attached screenshot(s) in this PR.