feat: Add dedicated contact page with routing and social links #566
Conversation
…al links Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
|
@mi-vaishnavi is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a dedicated contact page and ContactSection plus a reusable CardContact component, and updates header and footer navigation to include /contact while removing inline social icon links from the footer. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Footer.jsx (1)
4-6: 🛠️ Refactor suggestion | 🟠 MajorRemove unused FontAwesome imports.
FontAwesomeIcon,faEnvelope,faDiscord,faGithub,faGitlab, andfaTwitterare no longer referenced after the social icon links were removed. These dead imports may still contribute to bundle size depending on tree-shaking behavior.♻️ Proposed fix
-import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' -import { faEnvelope } from '@fortawesome/free-solid-svg-icons' -import { faDiscord, faGithub, faGitlab, faTwitter } from '@fortawesome/free-brands-svg-icons'
🤖 Fix all issues with AI agents
In `@src/components/CardContact.jsx`:
- Around line 46-48: CardContact currently does const Icon = iconMap[type] which
can be undefined for unsupported types; update CardContact to guard against
missing icons by resolving a fallback (e.g., iconMap['default'] or a
GenericIcon) or by rendering no icon when Icon is falsy before using <Icon />;
modify the Icon resolution in CardContact and update any usage of Icon so it is
only rendered when defined (and consider adding a PropTypes/defaultProps or
TypeScript check for the type prop) to prevent runtime render errors.
In `@src/components/ContactForm.jsx`:
- Around line 9-19: SectionHeading is currently destructuring "{ number,
children, className, ...props }" and spreading ...props onto the <h2>, which
causes eyebrow and title passed from ContactForm to become invalid DOM
attributes; update the SectionHeading component to destructure "eyebrow" and
"title" (e.g., { number, eyebrow, title, children, className, ...props }), stop
spreading eyebrow/title onto the <h2>, render eyebrow (as a small/eyebrow
element) and use title (or children) as the h2 content, and only spread the
remaining safe props onto the h2 to eliminate invalid attribute
warnings/tooltips.
🧹 Nitpick comments (3)
src/components/CardContact.jsx (1)
3-44: Consider extracting inline SVGs to a shared icons module or using an existing icon library.The footer previously used FontAwesome for these same icons (GitHub, GitLab, Discord, mail, Twitter/X). This component re-implements them as inline SVGs, creating duplicate icon representations across the codebase. If the project already ships FontAwesome, reusing it would reduce bundle size and maintenance surface.
src/components/ContactForm.jsx (2)
43-48:target="_blank"is unnecessary formailto:links.
mailto:links open the user's email client and don't navigate the browser, sotarget="_blank"has no meaningful effect (and in some browsers may cause a blank tab flash).♻️ Suggested fix
<CardContact href="mailto:contact@aossie.org" - target="_blank" title="Email" description="Contact the core team directly." type="mail" />
5-5: Component nameContactFormis misleading — there is no form.This component renders a contact links section and informational cards, not a form. A name like
ContactSectionorContactContentwould better convey its purpose.
Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/ContactForm.jsx`:
- Around line 9-16: SectionHeading in ContactForm.jsx currently contains a
paragraph-length description; since SectionHeading renders an <h2> you should
replace its long sentence with a short, semantic heading (e.g., "Get in Touch"
or "Contact") and move the descriptive sentence ("AOSSIE is an Australian
not-for-profit organization supporting innovation through open-source software.
Connect with us below.") out of SectionHeading into a separate paragraph element
(<p>) below the heading so the markup remains accessible and semantically
correct; update the JSX in ContactForm.jsx around the SectionHeading component
to reflect this change.
- Line 5: The component is misnamed: export default function ContactForm()
renders contact cards, not a form—rename the component to match its behavior
(e.g., change function ContactForm to ContactSection and export default
ContactSection) and update all imports/usages accordingly (search for
ContactForm references and replace with ContactSection), or alternatively
implement the actual responsive contact form inside the ContactForm component
(add form elements, state handlers, validation and submission logic) if you
intend to deliver issue `#565`; pick one approach and make the function name,
export, and all call sites consistent with that choice.
- Around line 40-46: Remove the unnecessary target="_blank" on the mailto link
usage of the CardContact component: in the JSX where CardContact is rendered
with href="mailto:contact@aossie.org" (the CardContact element), delete the
target prop so the browser handles the mailto protocol natively and avoids
opening a blank tab.
🧹 Nitpick comments (1)
src/components/ContactForm.jsx (1)
56-86: Insight cards section is well-structured.Keys are stable, dark/light mode classes are consistent with the rest of the design, and the layout is clean.
One minor note: the grid jumps directly from
grid-cols-1tosm:grid-cols-3, which may look cramped on smallersmscreens. Consider adding asm:grid-cols-2 lg:grid-cols-3progression for better readability at intermediate widths, matching the card grid above.
src/components/ContactForm.jsx
Outdated
| import { SectionHeading } from '@/components/SectionHeading' | ||
| import CardContact from '@/components/CardContact' | ||
|
|
||
| export default function ContactForm() { |
There was a problem hiding this comment.
Component name ContactForm is misleading — there is no form.
This component renders contact cards and informational sections but contains no form inputs or submission logic. The linked issue (#565) lists a "responsive contact form" as a deliverable. Either rename this to something like ContactSection to match its actual content, or implement the contact form as originally specified.
🤖 Prompt for AI Agents
In `@src/components/ContactForm.jsx` at line 5, The component is misnamed: export
default function ContactForm() renders contact cards, not a form—rename the
component to match its behavior (e.g., change function ContactForm to
ContactSection and export default ContactSection) and update all imports/usages
accordingly (search for ContactForm references and replace with ContactSection),
or alternatively implement the actual responsive contact form inside the
ContactForm component (add form elements, state handlers, validation and
submission logic) if you intend to deliver issue `#565`; pick one approach and
make the function name, export, and all call sites consistent with that choice.
src/components/ContactForm.jsx
Outdated
| <SectionHeading> | ||
| <span className="block text-lg font-semibold text-zinc-900 dark:text-zinc-100 mb-1"> | ||
| AOSSIE is an Australian not-for-profit organization supporting innovation through open-source software. | ||
| </span> | ||
| <span className="block text-base text-zinc-600 dark:text-zinc-400"> | ||
| Connect with us below. | ||
| </span> | ||
| </SectionHeading> |
There was a problem hiding this comment.
Semantic mismatch: paragraph-length content inside an <h2> element.
SectionHeading renders an <h2>. Placing a full descriptive sentence ("AOSSIE is an Australian not-for-profit…") inside a heading element is semantically incorrect — screen readers will announce this as a heading, and it doesn't function as one. Consider rendering the description outside of SectionHeading (e.g., in a <p>) and only keeping a short heading like "Contact" or "Get in Touch" inside it, which also aligns with the issue requirement for a hero header titled "Get in Touch."
🤖 Prompt for AI Agents
In `@src/components/ContactForm.jsx` around lines 9 - 16, SectionHeading in
ContactForm.jsx currently contains a paragraph-length description; since
SectionHeading renders an <h2> you should replace its long sentence with a
short, semantic heading (e.g., "Get in Touch" or "Contact") and move the
descriptive sentence ("AOSSIE is an Australian not-for-profit organization
supporting innovation through open-source software. Connect with us below.") out
of SectionHeading into a separate paragraph element (<p>) below the heading so
the markup remains accessible and semantically correct; update the JSX in
ContactForm.jsx around the SectionHeading component to reflect this change.
Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/ContactSection.jsx`:
- Around line 80-81: In ContactSection.jsx locate the heading element rendering
{item.title} (currently an <h4> inside the ContactSection component) and change
it to an <h3> so you do not skip a heading level between the existing <h2> and
this element; keep the same className ("text-sm font-semibold text-zinc-900
dark:text-zinc-100") and any props/ARIA attributes intact to preserve styling
and accessibility.
- Around line 10-14: The section currently renders two sibling headings: the
SectionHeading component (which outputs an <h2>) and a second literal <h2>;
change the second literal <h2> to a <p> to correct heading hierarchy while
keeping the same className string for styling (reference the SectionHeading
usage and the literal element with className "mt-6 text-2xl font-normal
leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl" so
you only swap the tag and preserve styles and content).
| <SectionHeading>Get in Touch</SectionHeading> | ||
|
|
||
| <h2 className="mt-6 text-2xl font-normal leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl"> | ||
| AOSSIE is an Australian not-for-profit organization supporting innovation through <span className="text-zinc-500 dark:text-zinc-400">open-source software.</span> | ||
| </h2> |
There was a problem hiding this comment.
Heading hierarchy: two sibling <h2> elements.
SectionHeading already renders an <h2> (line 10). The element on line 12 is also an <h2>, producing two peer <h2>s in the same section. The second element reads as descriptive body text, not a heading — switch it to a <p> (keeping the same styling classes).
Proposed fix
- <h2 className="mt-6 text-2xl font-normal leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl">
+ <p className="mt-6 text-2xl font-normal leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl">
AOSSIE is an Australian not-for-profit organization supporting innovation through <span className="text-zinc-500 dark:text-zinc-400">open-source software.</span>
- </h2>
+ </p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <SectionHeading>Get in Touch</SectionHeading> | |
| <h2 className="mt-6 text-2xl font-normal leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl"> | |
| AOSSIE is an Australian not-for-profit organization supporting innovation through <span className="text-zinc-500 dark:text-zinc-400">open-source software.</span> | |
| </h2> | |
| <SectionHeading>Get in Touch</SectionHeading> | |
| <p className="mt-6 text-2xl font-normal leading-relaxed tracking-tight text-zinc-900 dark:text-zinc-100 sm:text-3xl"> | |
| AOSSIE is an Australian not-for-profit organization supporting innovation through <span className="text-zinc-500 dark:text-zinc-400">open-source software.</span> | |
| </p> |
🤖 Prompt for AI Agents
In `@src/components/ContactSection.jsx` around lines 10 - 14, The section
currently renders two sibling headings: the SectionHeading component (which
outputs an <h2>) and a second literal <h2>; change the second literal <h2> to a
<p> to correct heading hierarchy while keeping the same className string for
styling (reference the SectionHeading usage and the literal element with
className "mt-6 text-2xl font-normal leading-relaxed tracking-tight
text-zinc-900 dark:text-zinc-100 sm:text-3xl" so you only swap the tag and
preserve styles and content).
| <h4 className="text-sm font-semibold text-zinc-900 dark:text-zinc-100"> | ||
| {item.title} |
There was a problem hiding this comment.
Skipped heading level: <h4> should be <h3>.
There is no <h3> in the document outline between the <h2> above and these <h4> elements. Skipping heading levels hurts screen-reader navigation and violates WCAG best practices. Use <h3> instead.
Proposed fix
- <h4 className="text-sm font-semibold text-zinc-900 dark:text-zinc-100">
+ <h3 className="text-sm font-semibold text-zinc-900 dark:text-zinc-100">
{item.title}
- </h4>
+ </h3>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <h4 className="text-sm font-semibold text-zinc-900 dark:text-zinc-100"> | |
| {item.title} | |
| <h3 className="text-sm font-semibold text-zinc-900 dark:text-zinc-100"> | |
| {item.title} | |
| </h3> |
🤖 Prompt for AI Agents
In `@src/components/ContactSection.jsx` around lines 80 - 81, In
ContactSection.jsx locate the heading element rendering {item.title} (currently
an <h4> inside the ContactSection component) and change it to an <h3> so you do
not skip a heading level between the existing <h2> and this element; keep the
same className ("text-sm font-semibold text-zinc-900 dark:text-zinc-100") and
any props/ARIA attributes intact to preserve styling and accessibility.
|
Hi @M4dhav, |
1 participant
Closes #565
Implemented a dedicated contact page with:
Hi @Zahnentferner,
Could you please review this PR The dedicated contact page is implemented with routing, communication cards, responsive contact form, and footer layout. Feedback on UI, responsiveness, or accessibility would be appreciated.
Thanks!
Summary by CodeRabbit