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.

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

Jump to bottom

Introduce new Slider shapes #147783

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Introduce new Slider shapes #147783

wants to merge 3 commits into from
+1,956 −60

Conversation

TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented May 3, 2024 •
edited

fixes Update Slider for Material 3 redesign

Description

This PR adds new Slider shapes BarSliderThumbShape, ExpressiveRoundedRectSliderTrackShape, and RoundedRectSliderValueIndicatorShape which are enabled when using the new SliderThemeData.use2024SliderShapes flag.

Preview

Thumb Pressed Thumb

New shapes when dragged

Light mode Dark mode
Record_2024-05-03-17-12-14.mp4
Record_2024-05-03-17-12-28.mp4

Stop indicator in the RoundedRectSliderValueIndicatorShape track shape

Preview 1 Preview 2
stop_indicator.mov
trackGapSize set to 0 Overridden trackGapSize & barThumbSize

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels May 3, 2024
@TahaTesser TahaTesser marked this pull request as ready for review May 3, 2024 15:42
@TahaTesser TahaTesser requested a review from Piinks May 3, 2024 17:13
guidezpl
guidezpl reviewed May 6, 2024
View reviewed changes
Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Nice work, just a general comment, I think we should make it easier to use the new shapes, since they will become the new default. Rather than manually specifying them, developers could flip a flag on ThemeData.

packages/flutter/lib/src/material/slider.dart Outdated Show resolved Hide resolved
@TahaTesser
Copy link
Member Author

Nice work, just a general comment, I think we should make it easier to use the new shapes, since they will become the new default. Rather than manually specifying them, developers could flip a flag on ThemeData.

Interesting idea, tho this is only specific to Slider (next RangeSlider). Maybe in the SliderThemeData? Do you've a name for this flag?

@guidezpl
Copy link
Member

guidezpl commented May 6, 2024

Perhaps useUpdatedSliderShapes or use2024SliderShapes? I think it would apply to all types of sliders

@TahaTesser
Copy link
Member Author

Perhaps useUpdatedSliderShapes or use2024SliderShapes? I think it would apply to all types of sliders

Added use2024SliderShapes flag, updated defaults and tests. I'll push the changes with updated docs tomorrow.

@TahaTesser TahaTesser requested a review from guidezpl May 8, 2024 15:03
guidezpl
guidezpl requested changes May 10, 2024
View reviewed changes
packages/flutter/lib/src/material/slider.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider_theme.dart Show resolved Hide resolved
packages/flutter/lib/src/material/slider_theme.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider_theme.dart Show resolved Hide resolved
packages/flutter/lib/src/material/slider_theme.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider_theme.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/slider.dart Outdated Show resolved Hide resolved
dev/tools/gen_defaults/lib/slider_template.dart Outdated Show resolved Hide resolved
@TahaTesser
Copy link
Member Author

@guidezpl
Appreciate the review! I'm about start traveling for Google I/O 2024. I'll address them after returning from states.

@TahaTesser TahaTesser mentioned this pull request May 22, 2024
Merged
9 tasks
@TahaTesser
Copy link
Member Author

TahaTesser commented May 22, 2024 •
edited

This PR #148789 removes the tokens for current M3 defaults so I will update the new shapes to use new the tokens and restore deleted token values as we plan to keep the current M3 shapes under the new flag.

@TahaTesser TahaTesser force-pushed the slider_new_shapes branch 3 times, most recently from bf932aa to 260bd1a Compare May 23, 2024 13:55
@TahaTesser
Copy link
Member Author

Updated the PR to use new tokens, restored the current M3 defaults, and made minor improvements.

guidezpl
guidezpl approved these changes May 24, 2024
View reviewed changes
Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

nice! looking for a second review

dev/tools/gen_defaults/lib/slider_template.dart
@override
Color? get inactiveTickMarkColor => ${componentColor('$tokenGroup.with-tick-marks.inactive.container')};
// TODO(tahatesser): Update this hard-coded value to use the correct token value.
Color? get inactiveTickMarkColor => _colors.primary;
Copy link
Member

Choose a reason for hiding this comment

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

I think this token is available

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked, it seems incorrect it uses secondary container color which makes inactive tick marks invisible on the inactive track. I will file an issue as this lands.

packages/flutter/lib/src/material/slider.dart Outdated
double? get trackGapSize => 6.0;
}

/// The default [SliderThemeData] for the legacy Material 3 slider.
Copy link
Member

Choose a reason for hiding this comment

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

nit: please clarify that legacy = first version of the M3 slider and that the look has been updated this year

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

packages/flutter/lib/src/material/slider_theme.dart
/// the default gap size is 6 pixels.
final double? trackGapSize;

/// A temporary flag that can be used to opt-in to the new 2024 slider shapes.
Copy link
Member

Choose a reason for hiding this comment

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

nit: please indicate that this flag is intended to migrate usages, after which it will be defaulted to true in a breaking change, and subsequently removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guidezpl guidezpl guidezpl approved these changes

@Piinks Piinks Awaiting requested review from Piinks

Assignees
No one assigned
Labels
d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update Slider for Material 3 redesign
2 participants
@TahaTesser @guidezpl