Fix(hero): align feature cards in hero section#46
Fix(hero): align feature cards in hero section#46shah0108 wants to merge 3 commits intoOpen-Source-Kashmir:mainfrom
Conversation
WalkthroughRemoved the redundant favicon link ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewersPoem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/components/Hero.css (1)
32-32: Consider using CSS custom properties for color values.Hardcoded color values appear throughout the stylesheet. For improved maintainability and theming support, consider extracting these into CSS variables at the
:rootlevel or component level.Example approach:
:root { --hero-text-primary: #ffffff; --hero-text-secondary: rgba(255, 255, 255, 0.85); --hero-accent: #2b6ef6; --hero-card-bg: rgba(11, 18, 33, 0.95); --button-primary-gradient: linear-gradient(90deg, #2b6ef6 0%, #4aa3ff 100%); } .hero-title { color: var(--hero-text-primary); } .hero-accent { color: var(--hero-accent); }Also applies to: 36-36, 40-40, 69-69, 106-106
src/components/Hero.jsx (2)
31-31: Remove redundantaria-hidden="false"attribute.The default value for
aria-hiddenisfalse, so explicitly setting it is unnecessary and adds noise to the markup.Apply this diff:
- <div className="hero-right" aria-hidden="false"> + <div className="hero-right">
32-47: Use semantic HTML instead of ARIA roles on generic divs.Using
<ul>and<li>elements would be more semantic and accessible than applyingrole="list"androle="listitem"to<div>elements.Apply this diff:
- <div className="feature-cards" role="list" aria-label="Highlights"> - <div className="card" role="listitem"> + <ul className="feature-cards" aria-label="Highlights"> + <li className="card"> <div className="card-icon" aria-hidden="true">🚀</div> <h3 className="card-title">Innovation</h3> - </div> + </li> - <div className="card" role="listitem"> + <li className="card"> <div className="card-icon" aria-hidden="true">💡</div> <h3 className="card-title">Learning</h3> - </div> + </li> - <div className="card" role="listitem"> + <li className="card"> <div className="card-icon" aria-hidden="true">🤝</div> <h3 className="card-title">Collaboration</h3> - </div> - </div> + </li> + </ul>You'll also need to add list reset styles to Hero.css:
.feature-cards { display: flex; flex-wrap: wrap; justify-content: center; gap: 1rem; list-style: none; padding: 0; margin: 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/favicon.pngis excluded by!**/*.png
📒 Files selected for processing (5)
LICENSE.md(1 hunks)README.md(1 hunks)index.html(1 hunks)src/components/Hero.css(1 hunks)src/components/Hero.jsx(1 hunks)
🔇 Additional comments (4)
index.html (1)
4-21: LGTM! SEO metadata is comprehensive.The addition of SEO metadata (description, keywords, Open Graph, Twitter Card) is well-structured and will improve the site's discoverability and social media sharing experience.
LICENSE.md (1)
1-21: LGTM! Standard MIT License.The MIT License text is correct and complete with proper copyright attribution to Open Source Kashmir.
src/components/Hero.css (1)
7-163: LGTM! Solid responsive layout implementation.The CSS provides a clean two-column layout with appropriate responsive breakpoints and smooth transitions. The hover effects and accessibility considerations are well-implemented.
src/components/Hero.jsx (1)
6-29: LGTM! Well-structured hero section with good accessibility practices.The component has a clear two-column layout with appropriate ARIA labels on the button group. The use of semantic HTML (
<header>,<h1>,<p>) and proper link attributes (target="_blank",rel="noopener noreferrer") demonstrates good practices.
src/components/Hero.jsx
Outdated
| <a className="btn primary" href="#programs">Explore Programs</a> | ||
| <a | ||
| className="btn outline" | ||
| href="https://github.com/Open-Source-Kashmir" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| Join GitHub | ||
| </a> | ||
| <a className="btn outline" href="#discord">Join Discord</a> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that anchor targets exist in the application.
The buttons link to hash fragments (#programs, #discord) that must exist as anchors elsewhere in the application for navigation to work correctly.
Run this script to check for matching anchor targets:
🏁 Script executed:
#!/bin/bash
# Search for id or name attributes matching the hash links
echo "Checking for #programs target..."
rg -n 'id=["'"'"']programs["'"'"']' --type=html --type=jsx --type=js
echo -e "\nChecking for #discord target..."
rg -n 'id=["'"'"']discord["'"'"']' --type=html --type=jsx --type=js
echo -e "\nIf no results above, these anchors need to be added to the page."Length of output: 187
🏁 Script executed:
#!/bin/bash
echo "Checking for #programs target..."
rg -n 'id=["'"'"']programs["'"'"']' -g '*.js' -g '*.jsx' -g '*.html'
echo -e "\nChecking for #discord target..."
rg -n 'id=["'"'"']discord["'"'"']' -g '*.js' -g '*.jsx' -g '*.html'
echo -e "\nIf no results above, these anchors need to be added to the page."Length of output: 131
Add missing in-page anchors for #programs and #discord
The hash links won’t work without matching id attributes—add id="programs" and id="discord" to the corresponding sections.
🤖 Prompt for AI Agents
In src/components/Hero.jsx around lines 18 to 27, the component adds in-page
links to "#programs" and "#discord" but the target sections are missing matching
id attributes; add id="programs" to the Programs section element and
id="discord" to the Discord section element (where those sections are rendered
in the codebase) so the anchor links scroll correctly, ensuring ids are unique
and placed on the top-level container or heading of each respective section.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
What I changed
Replaced src/components/Hero.jsx with a new hero component that uses a two-column layout (left: title/subtitle/buttons, right: feature cards).
Added src/components/Hero.css with scoped styles for the hero, button styles, and responsive feature card layout.
Why
The previous layout left the Innovation/Learning/Collaboration cards floating and uneven. This change aligns the cards in a structured flex container, improves spacing, and adds responsive behavior and accessible markup.
Files changed
->src/components/Hero.jsx — full replacement
->src/components/Hero.css — new file
How to test
->Title and subtitle render properly on the left.
->Buttons render and are clickable.
->Feature cards are centered and evenly spaced on the right at desktop widths.
->On narrow screens the layout stacks cleanly.
->Hovering a card produces a lift/shadow effect.
Notes
->If the project uses a different path for the Hero component, place these files in the same folder as the current Hero component and update the import path if necessary.
->No functional changes were made beyond structure and styling.
Thanks — krish
Closes #45
Summary by CodeRabbit