-
Notifications
You must be signed in to change notification settings - Fork 50
Fix(hero): align feature cards in hero section #46
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?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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