Skip to content

feat: added a method to assign an id to the Section component #30

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.

Sign up for GitHub

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

Open
kenmistry wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: added a method to assign an id to the Section component #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/app/globals.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@
}
}

html {
scroll-behavior: smooth;

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling scroll-behavior: smooth globally can cause motion-sickness and violates user preference for reduced motion. Add a @media (prefers-reduced-motion: reduce) override (e.g., revert to auto) so smooth scrolling is disabled for those users.

Suggested change
scroll-behavior: smooth;
scroll-behavior: smooth;
}
@media (prefers-reduced-motion: reduce) {
html {
scroll-behavior: auto;
}

Copilot uses AI. Check for mistakes.
}
9 changes: 8 additions & 1 deletion src/app/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default function Home() {
<Container>
<Hero />
<SectionTitle
sectionId="section-one"
preTitle="Nextly Benefits"
title=" Why should you use this landing page"
>
Expand All @@ -25,6 +26,7 @@ export default function Home() {
<Benefits imgPos="right" data={benefitTwo} />

<SectionTitle
sectionId="section-two"
preTitle="Watch a video"
title="Learn how to fullfil your needs"
>
Expand All @@ -36,6 +38,7 @@ export default function Home() {
<Video videoId="fZ0D0cnR88E" />

<SectionTitle
sectionId="section-three"
preTitle="Testimonials"
title="Here's what our customers said"
>
Expand All @@ -45,7 +48,11 @@ export default function Home() {

<Testimonials />

<SectionTitle preTitle="FAQ" title="Frequently Asked Questions">
<SectionTitle
sectionId="section-four"
preTitle="FAQ"
title="Frequently Asked Questions"
>
Answer your customers possible questions here, it will increase the
conversion rate as well as support or chat requests.
</SectionTitle>
Expand Down
2 changes: 2 additions & 0 deletions src/components/Container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import React from "react";

interface ContainerProps {
children: React.ReactNode;
idName?: string;
className?: string;
Comment on lines 3 to 6

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Container prop name idName is non-standard for a component that maps directly to a <div> attribute, and it makes call sites harder to read. Consider renaming this prop to id (or accepting React.HTMLAttributes<HTMLDivElement> / ...rest and spreading onto the <div>) to align with common React patterns and avoid a bespoke API surface.

Copilot uses AI. Check for mistakes.
}

export function Container(props: Readonly<ContainerProps>) {
return (
<div
id={props.idName || undefined}
className={`container p-8 mx-auto xl:px-0 ${
props.className ? props.className : ""
}`}>
Expand Down
2 changes: 2 additions & 0 deletions src/components/SectionTitle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import { Container } from "@/components/Container";

interface SectionTitleProps {
sectionId?: string;
preTitle?: string;
Comment on lines 4 to 6

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title/description mention adding an id to a Section component, but the change introduces sectionId on SectionTitle and an idName prop on Container. Please align the PR metadata with the actual components being changed (or rename/refactor to match the intended Section abstraction) to avoid confusion for maintainers.

Copilot uses AI. Check for mistakes.
title?: string;
align?: "left" | "center";
Expand All @@ -11,6 +12,7 @@ interface SectionTitleProps {
export const SectionTitle = (props: Readonly<SectionTitleProps>) => {
return (
<Container
idName={`${props.sectionId}`}

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idName={${props.sectionId}} stringifies undefined into the literal "undefined". Because Container treats any non-empty string as truthy, this results in <div id="undefined"> when sectionId isn’t provided. Pass props.sectionId through directly (no template literal) and/or only set the prop when defined.

Suggested change
idName={`${props.sectionId}`}
idName={props.sectionId}

Copilot uses AI. Check for mistakes.
className={`flex w-full flex-col mt-4 ${
props.align === "left" ? "" : "items-center justify-center text-center"
}`}>
Expand Down
Loading