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

Fix discrete Slider and RangeSlider to enforce thumb height padding when the track shape is non-rounded #164703

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

Conversation

TahaTesser
Copy link
Member

Fixes Discrete Slider and RangeSlider applies thumb padding when using custom Slider shapes

Code Sample

expand to view the code sample
import 'package:flutter/material.dart';

void main() => runApp(const RangeSliderExampleApp());

class RangeSliderExampleApp extends StatelessWidget {
  const RangeSliderExampleApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      theme: ThemeData(
        sliderTheme: const SliderThemeData(
          trackHeight: 32,
          trackShape: RectangularSliderTrackShape(),
          rangeTrackShape: RectangularRangeSliderTrackShape(),
          thumbColor: Colors.amber,
        ),
      ),
      home: Scaffold(
        body: Column(
          spacing: 20.0,
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Slider(
              value: 100,
              max: 100,
              divisions: 100,
              onChanged: (double value) {},
            ),
            RangeSlider(
              values: const RangeValues(0, 100),
              max: 100,
              divisions: 100,
              onChanged: (RangeValues values) {},
            ),
          ],
        ),
      ),
    );
  }
}

Before

Screenshot 2025-03-06 at 13 33 17

After

Screenshot 2025-03-06 at 13 33 28

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. labels Mar 6, 2025
@TahaTesser TahaTesser marked this pull request as ready for review March 6, 2025 14:26
@TahaTesser TahaTesser requested a review from QuncCccccc March 10, 2025 11:45
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

This LGTM. Thanks for the fix. Just a little confused about the numbers in the test.

material,
paints
// Start thumb.
..circle(x: sliderPadding, y: 300.0, color: theme.colorScheme.primary)
Copy link
Contributor

@QuncCccccc QuncCccccc Mar 11, 2025

Choose a reason for hiding this comment

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

Just a question, how do we get this 24? It looks like sliderPadding isn't used anywhere.

Copy link
Member Author

@TahaTesser TahaTesser Mar 12, 2025

Choose a reason for hiding this comment

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

This comes from the track padding applied by base track shape in RangeSlider widget.

Here trackLeft and trackRight are 24 pixels.

final double trackLeft = offset.dx + math.max(overlayWidth / 2, thumbWidth / 2);
final double trackTop = offset.dy + (parentBox.size.height - trackHeight) / 2;
final double trackRight = trackLeft + parentBox.size.width - math.max(thumbWidth, overlayWidth);
final double trackBottom = trackTop + trackHeight;

I made such track padding customizable in the Slider widget last year (#156143) but it's not yet implemented in RangeSlider but the I used the same wording here "slider padding" since it's effectively the same thing.

I just filed an issue #165046 and assigned. I will complete the Slider padding work for RangeSlider. Thanks for the reminder.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see! Could you add a comment above the number? Thanks a lot for the explanation!

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment

@TahaTesser TahaTesser requested a review from QuncCccccc March 12, 2025 11:23
@TahaTesser TahaTesser force-pushed the fix_sliders_discrete_padding branch 2 times, most recently from 3e364c5 to 3b67573 Compare March 17, 2025 08:22
@QuncCccccc
Copy link
Contributor

The failed Google testing seems related to M2 style.

@TahaTesser
Copy link
Member Author

TahaTesser commented Mar 18, 2025

@QuncCccccc
Thanks! I've pushed which a commit restores like discrete padding the way it was before the this PR and renamed the same discrete padding in RangeSlider to be consistent.

Looks like checks are green. Could you please confirm this fixes the issue you described?

@TahaTesser TahaTesser force-pushed the fix_sliders_discrete_padding branch from 1bb820b to 6a39a39 Compare March 19, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discrete Slider and RangeSlider applies thumb padding when using custom Slider shapes
2 participants