boeledi / RangeSlider

RangeSlider Widget for Flutter
Other
374 stars 67 forks source link

Thumbs collide without exploring all range possibilities #8

Closed tevanc14 closed 5 years ago

tevanc14 commented 5 years ago

If both sliders are moved toward the same position with a small amount of discrete divisions, the thumbs will overlap as expected.

If the same is done with a larger number of divisions (enough so that the overlays collide) the thumbs will not overlap.

Continuous slider thumbs never overlap and, if the range is large enough, will not even explore all of the possibilities of the slider.

I am unsure if the lack of continuous overlapping is by design or not, but I believe the discrete behavior inconsistencies to be a bug. Upon initial observations, collision seems to be determined upon if the thumb shapes themselves are colliding, which causes problems when the divisions are smaller than the size of the thumb shape.

A demonstration of this behavior derived from your example is below:

import 'package:flutter/material.dart';
import 'package:flutter_range_slider/flutter_range_slider.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'RangeSlider Collision Bug',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: RangeSliderSample(),
    );
  }
}

class RangeSliderSample extends StatefulWidget {
  @override
  _RangeSliderSampleState createState() => _RangeSliderSampleState();
}

class _RangeSliderSampleState extends State<RangeSliderSample> {
  double _goodLowerValue = 1.0;
  double _goodUpperValue = 11.0;

  double _badLowerValue = 1.0;
  double _badUpperValue = 12.0;

  double _continuousLowerValue = 1.0;
  double _continuousUpperValue = 50.0;

  @override
  void initState() {
    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    return SafeArea(
      top: false,
      bottom: false,
      child: Scaffold(
        appBar: AppBar(
          title: Text('RangeSlider Demo'),
        ),
        body: Container(
          padding: const EdgeInsets.only(
            top: 50.0,
            left: 10.0,
            right: 10.0,
          ),
          child: Column(
            children: <Widget>[
              Text(
                'Good',
                textAlign: TextAlign.center,
              ),
              Row(
                children: <Widget>[
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_goodLowerValue.toInt()}',
                    ),
                  ),
                  Expanded(
                    child: RangeSlider(
                      min: 1.0,
                      max: 11.0,
                      lowerValue: _goodLowerValue,
                      upperValue: _goodUpperValue,
                      divisions: 11,
                      showValueIndicator: true,
                      valueIndicatorMaxDecimals: 0,
                      onChanged: (double newLowerValue, double newUpperValue) {
                        setState(() {
                          _goodLowerValue = newLowerValue;
                          _goodUpperValue = newUpperValue;
                        });
                      },
                      onChangeStart:
                          (double startLowerValue, double startUpperValue) {
                        print(
                            'Good - Started with values: $startLowerValue and $startUpperValue');
                      },
                      onChangeEnd:
                          (double newLowerValue, double newUpperValue) {
                        print(
                            'Good - Ended with values: $newLowerValue and $newUpperValue');
                      },
                    ),
                  ),
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_goodUpperValue.toInt()}',
                    ),
                  ),
                ],
              ),
              Text(
                'Bad',
                textAlign: TextAlign.center,
              ),
              Row(
                children: <Widget>[
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_badLowerValue.toInt()}',
                    ),
                  ),
                  Expanded(
                    child: RangeSlider(
                      min: 1.0,
                      max: 12.0,
                      lowerValue: _badLowerValue,
                      upperValue: _badUpperValue,
                      divisions: 12,
                      showValueIndicator: true,
                      valueIndicatorMaxDecimals: 0,
                      onChanged: (double newLowerValue, double newUpperValue) {
                        setState(() {
                          _badLowerValue = newLowerValue;
                          _badUpperValue = newUpperValue;
                        });
                      },
                      onChangeStart:
                          (double startLowerValue, double startUpperValue) {
                        print(
                            'Bad - Started with values: $startLowerValue and $startUpperValue');
                      },
                      onChangeEnd:
                          (double newLowerValue, double newUpperValue) {
                        print(
                            'Bad - Ended with values: $newLowerValue and $newUpperValue');
                      },
                    ),
                  ),
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_badUpperValue.toInt()}',
                    ),
                  ),
                ],
              ),
              Text(
                'Continuous',
                textAlign: TextAlign.center,
              ),
              Row(
                children: <Widget>[
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_continuousLowerValue.toInt()}',
                    ),
                  ),
                  Expanded(
                    child: RangeSlider(
                      min: 1.0,
                      max: 50.0,
                      lowerValue: _continuousLowerValue,
                      upperValue: _continuousUpperValue,
                      showValueIndicator: true,
                      valueIndicatorMaxDecimals: 0,
                      onChanged: (double newLowerValue, double newUpperValue) {
                        setState(() {
                          _continuousLowerValue = newLowerValue;
                          _continuousUpperValue = newUpperValue;
                        });
                      },
                      onChangeStart:
                          (double startLowerValue, double startUpperValue) {
                        print(
                            'Continuous - Started with values: $startLowerValue and $startUpperValue');
                      },
                      onChangeEnd:
                          (double newLowerValue, double newUpperValue) {
                        print(
                            'Continuous - Ended with values: $newLowerValue and $newUpperValue');
                      },
                    ),
                  ),
                  SizedBox(
                    width: 28.0,
                    child: Text(
                      '${_continuousUpperValue.toInt()}',
                    ),
                  ),
                ],
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Let me know if any part of this is unclear, thanks.

lucaslcode commented 5 years ago

Also having this problem. I understand it will cause a difficult design issue - when they overlap, how do you determine if the lower or upper thumb is tapped?

One possibility is to remove the distinction and just have whichever has the lower value to be the lower thumb, but that would require a new method of determining which is currently being dragged.

boeledi commented 5 years ago

Hi, I just released version 1.1.0 which now "normally" corrects this behavior.