Skip to content
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

Carousel: Separate draggable area from controls #33155

Conversation

Mitch-At-Work
Copy link
Contributor

@Mitch-At-Work Mitch-At-Work commented Oct 28, 2024

Previous Behavior

When 'draggable' was enabled on carousel the draggable area included anything in the main Carousel wrapper

New Behavior

Draggable now only enables draggable area within the CarouselSlider window

This aligns with the reccomended Embla pattern for draggable carousels:
https://www.embla-carousel.com/guides/previous-and-next-buttons/

@Mitch-At-Work Mitch-At-Work marked this pull request as ready for review October 28, 2024 17:59
@Mitch-At-Work Mitch-At-Work requested review from a team as code owners October 28, 2024 17:59
@Mitch-At-Work Mitch-At-Work force-pushed the user/mifraser/carousel-drag-controls branch 2 times, most recently from 9086b34 to 2c074fb Compare October 29, 2024 16:47
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 29, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.16 MB
290.619 kB
1.161 MB
290.83 kB
1.175 kB
211 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
global-context
createContext
510 B
328 B
global-context
createContextSelector
537 B
339 B
react-accordion
Accordion (including children components)
106.728 kB
32.718 kB
react-avatar
Avatar
49.303 kB
15.815 kB
react-avatar
AvatarGroup
20.106 kB
7.968 kB
react-avatar
AvatarGroupItem
63.447 kB
20.034 kB
react-badge
Badge
25.954 kB
8.595 kB
react-badge
CounterBadge
26.733 kB
8.872 kB
react-badge
PresenceBadge
25.719 kB
9.457 kB
react-breadcrumb
@fluentui/react-breadcrumb - package
114.291 kB
31.695 kB
react-button
Button
37.174 kB
10.803 kB
react-button
CompoundButton
43.588 kB
12.101 kB
react-button
MenuButton
41.989 kB
12.144 kB
react-button
SplitButton
50.006 kB
13.716 kB
react-button
ToggleButton
53.107 kB
12.561 kB
react-calendar-compat
Calendar Compat
150.095 kB
40.026 kB
react-card
Card - All
101.77 kB
28.772 kB
react-card
Card
94.544 kB
26.951 kB
react-card
CardFooter
14.355 kB
5.79 kB
react-card
CardHeader
16.888 kB
6.669 kB
react-card
CardPreview
14.42 kB
5.922 kB
react-checkbox
Checkbox
35.118 kB
12.077 kB
react-combobox
Combobox (including child components)
104.383 kB
34.197 kB
react-combobox
Dropdown (including child components)
104.996 kB
34.122 kB
react-components
react-components: Button, FluentProvider & webLightTheme
69.21 kB
20.174 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
220.616 kB
63.902 kB
react-components
react-components: FluentProvider & webLightTheme
44.447 kB
14.59 kB
react-datepicker-compat
DatePicker Compat
224.165 kB
63.423 kB
react-dialog
Dialog (including children components)
100.297 kB
30.076 kB
react-divider
Divider
21.328 kB
7.953 kB
react-field
Field
23.399 kB
8.898 kB
react-image
Image
15.36 kB
6.236 kB
react-input
Input
28.014 kB
9.444 kB
react-jsx-runtime
Classic Pragma
1.057 kB
530 B
react-jsx-runtime
JSX Dev Runtime
3.771 kB
1.643 kB
react-jsx-runtime
JSX Runtime
4.367 kB
1.874 kB
react-label
Label
14.671 kB
5.99 kB
react-link
Link
17.326 kB
7.032 kB
react-list-preview
List
89.148 kB
26.597 kB
react-list-preview
ListItem
112.707 kB
33.428 kB
react-menu
Menu (including children components)
152.661 kB
46.116 kB
react-menu
Menu (including selectable components)
155.342 kB
46.598 kB
react-message-bar
MessageBar (all components)
24.878 kB
9.264 kB
react-motion
@fluentui/react-motion - createMotionComponent()
4.303 kB
1.899 kB
react-motion
@fluentui/react-motion - createPresenceComponent()
5.038 kB
2.229 kB
react-motion
@fluentui/react-motion - PresenceGroup
1.714 kB
819 B
react-overflow
hooks only
12.808 kB
4.819 kB
react-persona
Persona
56.194 kB
17.695 kB
react-popover
Popover
128.904 kB
40.314 kB
react-portal
Portal
14.563 kB
5.118 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
react-positioning
usePositioning
27.057 kB
9.698 kB
react-progress
ProgressBar
17.084 kB
6.891 kB
react-provider
FluentProvider
24.623 kB
8.893 kB
react-radio
Radio
32.672 kB
10.343 kB
react-radio
RadioGroup
15.762 kB
6.423 kB
react-select
Select
27.732 kB
10.124 kB
react-slider
Slider
37.169 kB
12.538 kB
react-spinbutton
SpinButton
36.06 kB
11.815 kB
react-spinner
Spinner
25.245 kB
8.539 kB
react-swatch-picker
@fluentui/react-swatch-picker - package
104.258 kB
30.231 kB
react-switch
Switch
35.319 kB
11.314 kB
react-table
DataGrid
161.034 kB
45.71 kB
react-table
Table (Primitives only)
42.666 kB
13.854 kB
react-table
Table as DataGrid
131.869 kB
36.57 kB
react-table
Table (Selection only)
70.536 kB
19.999 kB
react-table
Table (Sort only)
69.179 kB
19.61 kB
react-tag-picker
@fluentui/react-tag-picker - package
184.091 kB
55.425 kB
react-tags
InteractionTag
15.199 kB
6.157 kB
react-tags
Tag
29.016 kB
9.524 kB
react-tags
TagGroup
82.719 kB
24.524 kB
react-text
Text - Default
17.061 kB
6.723 kB
react-text
Text - Wrappers
20.242 kB
7.048 kB
react-textarea
Textarea
26.572 kB
9.755 kB
react-timepicker-compat
TimePicker
107.372 kB
35.755 kB
react-toast
Toast (including Toaster)
98.338 kB
29.591 kB
react-tooltip
Tooltip
55.517 kB
19.515 kB
react-utilities
SSRProvider
180 B
160 B
🤖 This report was generated against 49e6794b31d8097ace17a179a58d4db373a47ca4

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

IMO the current change in the structure does not scale 😐

I played with it a bit and I propose to try the following plan:

  1. Undo all changes & deprecate CarouselSlider
  2. Create new CarouselViewport component that will have root (i.e. embla__viewport) & slider (better name? i.e. embla__container) slots
  3. Undo changes in useEmblaCarousel()
  4. Rename containerRef in useEmblaCarousel() to viewportRef, add viewportRef to CarouselContext and consume it in CarouselViewport. This way we avoid .querySelector() in useEmblaCarousel()
  5. Ensure that this still somehow works with deprecated CarouselSlider

@Mitch-At-Work Mitch-At-Work force-pushed the user/mifraser/carousel-drag-controls branch from 033ffe1 to 259922a Compare October 30, 2024 18:21
@Mitch-At-Work Mitch-At-Work requested a review from a team as a code owner October 30, 2024 18:21
@Mitch-At-Work Mitch-At-Work requested a review from a team as a code owner November 1, 2024 18:05
@Mitch-At-Work Mitch-At-Work merged commit 69dc72a into microsoft:master Nov 1, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants