-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(carousel): Add stories for numeric, alphabetical, and carousel pagination variants #35174
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: master
Are you sure you want to change the base?
feat(carousel): Add stories for numeric, alphabetical, and carousel pagination variants #35174
Conversation
d822d61
to
85c3eeb
Compare
📊 Bundle size report✅ No changes found |
Pull request demo site: URL |
85c3eeb
to
4a1b05e
Compare
{index + 1} | ||
</button> | ||
); | ||
}; |
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.
I am not a big fan of this approach it also breaks accessibility on CarouselNavButton
(not the same role being applied):

Let's use recomposition instead, this will be more solid and future proof:
const CustomNavButton: ForwardRefComponent<CarouselNavButtonProps> = React.forwardRef((props, ref) => {
const classes = useCustomNavButtonClasses();
const state = useCarouselNavButton_unstable(props, ref);
state.root.children = state.index;
state.root.className = mergeClasses(classes.root, state.selected && classes.selected, state.root.className);
return renderCarouselNavButton_unstable(state);
});
// packages/react-components/react-carousel/library/src/components/CarouselNavButton/useCarouselNavButton.ts
const state: CarouselNavButtonState = {
selected,
appearance,
components: {
root: 'button',
},
root: _carouselButton,
+ index
};
// packages/react-components/react-carousel/library/src/components/CarouselNavButton/CarouselNavButton.types.ts
/**
* Enables selection state control
*/
selected?: boolean;
+ index?: number;
</CarouselSlider> | ||
</CarouselViewport> | ||
|
||
<CarouselNav className={classes.nav}>{index => <CustomNavButton index={index} />}</CarouselNav> |
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.
|
||
return ( | ||
<div className={classes.paginationControls}> | ||
<Input |
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.
Let's use SpinButton
instead: https://storybooks.fluentui.dev/react/?path=/docs/components-spinbutton--docs
@@ -0,0 +1,184 @@ | |||
import { |
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.
Do we really need to have this example this way? Have to say that UI has mixed user experience to suggest it to anybody...
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.
Please re-implement buttons & let's follow up on the pagination example in offline
The design review highlighted that dot indicators alone don’t cover all carousel navigation needs.
This PR expands Fluent UI Storybook with numeric, alphabetical, and pagination CarouselNav stories, providing developers with flexible, ready-to-use navigation patterns.
Previous Behavior
Storybook only included the default CarouselNav example, limiting discoverability of other navigation patterns. Developers often had to experiment or build their own demos to explore alternative configurations.
New Behavior
New Stories Added