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

Slider crashes when Minimum value greater than zero and set before Max value in XAML #1943

Open
Paul-Brenner-Tangoe opened this Issue Feb 21, 2018 · 3 comments

Comments

6 participants
@Paul-Brenner-Tangoe

Paul-Brenner-Tangoe commented Feb 21, 2018

Description

Migrating https://bugzilla.xamarin.com/show_bug.cgi?id=43435 here

The difference between the working and crashing versions is the order of the Minimum and Maximum properties in the XAML. Note that if Minimum is set to zero, the order is no longer an issue.

Steps to Reproduce

Run app, note that it works fine
Uncomment line #8 in MainPage.xaml
Run app, note it crashes with "System.ArgumentException: Value was an invalid value for Minimum"

Expected Behavior

App does not crash

Actual Behavior

App crashes

Basic Information

  • Version with issue: 2.5.0.280555
  • Last known good version: N/A
  • IDE: VS2017
  • Platform Target Frameworks:
    • iOS: yes
    • Android: yes
    • UWP: not tested
  • Android Support Library Version: 25
  • Nuget Packages: just forms
  • Affected Devices: tested on Nexus 5x running Android 7, and iPhone 5s simulator running iOS 9.3

Reproduction Link

https://www.dropbox.com/s/qdweqnvh1hsj4wh/slidercrashsample.zip?dl=0

@pauldipietro pauldipietro added this to New in Triage Feb 21, 2018

@CZEMacLeod

This comment has been minimized.

CZEMacLeod commented Feb 22, 2018

I had a similar issue in pure code with something like

Slider s = new Slider() { Minimum = 1000, Maximum = 2000 };

I had to change it to

Slider s = new Slider() { Maximum = 2000, Minimum = 1000 };

I have a feeling that the blocks/errors should not be raised until it is fully initialized.
Also what if you want a slider going from 100->0?

@samhouts samhouts added the t/bug 🐛 label Feb 22, 2018

@samhouts samhouts moved this from New to Ready For Work in Triage Feb 22, 2018

@sayedihashimi

This comment has been minimized.

sayedihashimi commented Feb 23, 2018

A customer I'm working with has also run into this. He said it took him about an hour to figure out he needed to set Maximum before Minimum. Also he stated that when developing in XAML with UWP there was no such requirement for Maximum to be before Minimum.

@samhouts samhouts added the i/critical label Feb 24, 2018

@samhouts samhouts added this to In Progress in vNext+1 (master) May 21, 2018

@samhouts samhouts moved this from In Progress to Done in vNext+1 (master) Jun 4, 2018

@samhouts samhouts moved this from Done to In Progress in vNext+1 (master) Jun 11, 2018

@samhouts samhouts removed this from Ready For Work in Triage Jun 12, 2018

@samhouts samhouts moved this from In Progress to Done in vNext+1 (master) Jun 26, 2018

@samhouts samhouts moved this from Done to In Progress in vNext+1 (master) Jun 26, 2018

@samhouts samhouts added the e/7 🕖 label Jul 26, 2018

@charlesroddie

This comment has been minimized.

charlesroddie commented Oct 19, 2018

The current behavior is absurd. https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/slider
To set (Minimum, Maximum) = (2,3), you have to set maximum first, then minimum.
To set (Minimum, Maximum) = (-3,-2), you have to set minimum first, then maximum.

My suggestion (from #2765 (comment)):

We have to be able to set Minimum, Value and Maximum separately. It was a mistake to allow this (there should be a single float*float*float property instead of three float properties), but we are stuck with it.

Setting Min, Value, Max should set internal mutable variables reqMin, reqVal, reqMax. Getting Value, Min, Max gets the actual values, which are a function of the these req values. This would not prevent temporary glitches in Min, Value, Max but it would prevent these glitches from crashing the UI (since the UI always gets valid inputs). Glitch-free operation can be done with an extra property containing all three values.

type SliderValues(minimum:float, value:float, maximum:float) =
    member t.Minimum = Minimum
    member t.Value = value
    member t.Maximum = maximum

type Xamarin.Forms.Slider =
    ...
    member t.Minimum
        with get() =
            if reqMin <= reqVal && reqVal <= reqMax
            then reqMin
            else 0. // A valid temporary value; could also use min(reqMin,reqVal,reqMax) if the temporary value needs to be a better guess
        and set v = reqMin <- v
    member t.Value
        with get() =
            if reqMin <= reqVal && reqVal <= reqMax
            then reqVal
            else 0.5 // Or min(max(t.Minimum,reqVal),t.Maximum)
        and set v = reqVal <- v
    member t.Maximum
        with get() =
            if reqMin <= reqVal && reqVal <= reqMax
            then reqMax
            else 1. // Or max(reqMin,reqVal,reqMax)
        and set v = reqMax <- v

    // set all values in a glitch free way with proper error handling
    member t.SliderValues
        with get() = SliderValues(t.Minimum, t.Value, t.Maximum)
        and set (v:SliderValues) =
            if v.Minimum <= v.Value && v.Value <= v.Maximum
            then SliderValues(v.Minimum, v.Value, v.Maximum)
            else failwith "Incorrectly ordered slider values"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment