Skip to content

fix: navigation component #1082

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
tomasvn wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: navigation component #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3518dfe
fix: navigation component
tomasvn Oct 30, 2024
a391411
fix(PageIndicators.tsx): add extra text
tomasvn Oct 30, 2024
653a62d
fix(README.md): update install section
tomasvn Oct 30, 2024
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
5 changes: 5 additions & 0 deletions .changeset/hip-jobs-change.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'nuka-carousel': patch
---

Replace div for button elements #1081
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The changeset summary is a bit ungrammatical (“Replace div for button elements”). Consider updating it to something like “Replace div elements with button elements (#1081)” so the release note reads cleanly.

Suggested change
Replace div for button elements #1081
Replace div elements with button elements (#1081)

Copilot uses AI. Check for mistakes.
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ $ pnpm run build

## Running the demos

Run the NextJS example on `localhost:3000`
Run docosaurus site on `localhost:3000`

```sh
$ pnpm run start:nextjs
$ pnpm run start:website
```

To make changes to the Nuka Carousel library and have those changes reflect in the NextJS demo app also run
To make changes to the Nuka Carousel library and have those changes reflect in the Docosaurus demo app also run

Comment on lines +20 to 27
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

“docosaurus” / “Docosaurus” appears to be a misspelling of “Docusaurus”. Updating this improves clarity and avoids propagating the typo in contributor docs.

Copilot uses AI. Check for mistakes.
```sh
$ pnpm run build:watch
Expand Down
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,24 @@ Small, fast and accessibility-first React carousel library with easily customiza

To add `nuka-carousel` to your project run the following command in your project folder.

#### Yarn

```bash
$ yarn add nuka-carousel
```

#### NPM

```bash
$ npm install nuka-carousel
```

#### PNPM

```bash
$ pnpm add nuka-carousel
```

Come learn more and see a live demo at our [docs site](https://commerce.nearform.com/open-source/nuka-carousel)!

## Support
Expand Down
7 changes: 4 additions & 3 deletions packages/nuka/src/Carousel/Carousel.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
border-radius: 9999px;
font-size: 1rem;
user-select: none;
border: 0;
display: flex;
align-items: center;
justify-content: center;
}
Comment on lines +44 to 47
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

.nuka-nav-button declares both display: none and later display: flex, so the display: none is overridden and nav buttons will render even when they are not “enabled”. At the same time, the .nuka-nav-button-enabled rule was removed but the class is still used (and referenced by the auto-hide hover selector), so arrow visibility logic is inconsistent. Reintroduce a .nuka-nav-button-enabled rule (using display: flex) and remove the conflicting display in the base rule so enabled/disabled + hover behavior works as intended.

Suggested change
display: flex;
align-items: center;
justify-content: center;
}
align-items: center;
justify-content: center;
}
.nuka-nav-button-enabled {
display: flex;
}

Copilot uses AI. Check for mistakes.
.nuka-nav-button.nuka-nav-button-prev {
left: 0;
Expand All @@ -52,9 +56,6 @@
.nuka-nav-button:hover {
background-color: rgba(146, 148, 151, 0.65);
}
.nuka-nav-button-enabled {
display: block;
}
.nuka-container-auto-hide .nuka-nav-button {
display: none;
}
Expand Down
13 changes: 13 additions & 0 deletions packages/nuka/src/Carousel/Carousel.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,16 @@ export const AfterSlide: Story = {
),
},
};

export const WithArrows: Story = {
args: {
showArrows: true,
children: (
<>
{[...Array(10)].map((_, index) => (
<ExampleSlide key={index} index={index} />
))}
</>
),
},
};
18 changes: 14 additions & 4 deletions packages/nuka/src/Carousel/NavButtons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ export function NavButtons() {

return (
<>
<div className={prevNavClassName} onClick={goBack}>
<button
type="button"
className={prevNavClassName}
onClick={goBack}
aria-label="Go to previous slide"
>
Comment on lines +27 to +32
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

enablePrevNavButton/enableNextNavButton are computed but not reflected in the buttons’ interactive state. These buttons remain clickable/focusable even when navigation should be disabled at the ends (wrapMode="nowrap"). Consider wiring these booleans to disabled (and matching styles) and/or conditionally omitting the onClick so button semantics match the enabled state.

Copilot uses AI. Check for mistakes.
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
Expand All @@ -36,8 +41,13 @@ export function NavButtons() {
clipRule="evenodd"
/>
</svg>
</div>
<div className={nextNavClassName} onClick={goForward}>
</button>
<button
type="button"
className={nextNavClassName}
onClick={goForward}
aria-label="Go to next slide"
>
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
Expand All @@ -49,7 +59,7 @@ export function NavButtons() {
clipRule="evenodd"
/>
</svg>
</div>
</button>
</>
);
}
2 changes: 1 addition & 1 deletion packages/nuka/src/Carousel/PageIndicators.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const PageIndicators = () => {
onClick={() => goToPage(index)}
className={className(index)}
>
<span className="nuka-hidden">{index + 1}</span>
<span className="nuka-hidden">Slide {index + 1}</span>
</button>
))}
</div>
Expand Down