nativescript-community / ui-material-components

Monorepo that contains all of the NativeScript Material Design plugins.
https://nativescript-community.github.io/ui-material-components/
Apache License 2.0
218 stars 80 forks source link

iOS Slider Discrete Values Issue #394

Open dangrima90 opened 2 years ago

dangrima90 commented 2 years ago

I think I've found a bug with how the numberOfDiscreteValues is being calculated for iOS. I'm currently using @nativescript-community/ui-material-slider@7.0.20

I have the following example:

Min Value: 1000 Max Value: 10000 Step Size: 1000

When debugging I've noticed that numberOfDiscreteValues is being to set to 9 instead of 10. This is resulting in a continuous slider rather than a discrete one. From what I'm seeing the calculation is a difference of max and min, rather than the number of values.

Here the calculation:

https://github.com/nativescript-community/ui-material-components/blob/f889849cd6c37c56cebe7d1ce71577e9912237b4/src/slider/slider.ios.ts#L71-L76

https://github.com/nativescript-community/ui-material-components/blob/f889849cd6c37c56cebe7d1ce71577e9912237b4/src/slider/slider.ios.ts#L80-L88

With the above logic the numberOfDiscreteValues is one less than the correct value.

I'm not blocked with the issue as I'm setting the numberOfDiscreteValues myself. I'm using NativeScript-Vue, here's a sample logic that is working for me:

computed: {
  totalTickMarks() {
    /**
     * adding one as the number of ticks represents the number of values and not the difference
     * Example Range: 0 to 10 = 11 ticks; Therefore: 10 - 0 + 1 = 11
     * Example Range: 5 to 50 = 46 ticks; Therefore: 50 - 5 + 1 = 46
     */
    const tickMarksCount = this.maxValue - this.minValue + 1;

    if (this.stepSize) {
      // always set value to whole number after dividing
      return Math.ceil(tickMarksCount / this.stepSize);
    }

    return tickMarksCount;
  }
},

// ...

methods: {
  sliderLoad() {
    const nativeView = this.$refs.slider.nativeView.ios;
    nativeView.numberOfDiscreteValues = this.totalTickMarks;
  },
},
farfromrefug commented 2 years ago

@dangrima90 could you create a PR?

dangrima90 commented 2 years ago

@farfromrefug sure I'll give it a go :)

dangrima90 commented 2 years ago

@farfromrefug here's the PR https://github.com/nativescript-community/ui-material-components/pull/395

As mentioned in the PR description unfortunately I didn't manage to get the demos working on my machine, however I tested the same code in my local project and it's working fine.

brianrclow commented 2 years ago

@farfromrefug @dangrima90 I see this issue is still open and after using the slider with the most up-to-date version I'm not sure if the PR #395 that was merged resolves everything.

I've created a demo repo that uses Nativescript Angular(I'd imagine it's the same issue with any flavor) to reproduce the issue: https://github.com/brianrclow/slider-initial-value-issue

The issue is the numberOfDiscreteValues being 1 more than expected for some min/max values does not set the initial "dot" in the correct spot on the slider. In my demo repo, I show this issue with 2 sliders.

Slider 1 Min: 0 Max: 100 Value: 50 Step: 10

Outcome: slider initial dot position is in the middle at the "50" tic mark position.

Slider 2 Min: 20 Max: 100 Value: 50 Step: 10

Outcome: slider initial dot position is in the middle at the "60" tic mark position not where I would expect it to be at the "50" tic mark position, 1 to the left, off center.

I'm not entirely sure how to best solve this as it seems removing the "+1" that was added in the PR solves it. Taking const valuesCount = this.maxValue - this.minValue + 1; to const valuesCount = this.maxValue - this.minValue; seems to solve it but I don't know enough about the codebase to know if this messes with other things. It's possible there are more checks or a better way to calculate the numberOfDiscreteValues needed that takes into account varying min values other than 0.

Interested in your thoughts and if you've come to a similar understanding. Thanks.

Here is a screenshot of the app running: Simulator Screen Shot - iPhone 14 Pro - 2022-11-08 at 17 15 40

brianrclow commented 2 years ago

Here is a video of it in an app I'm working on: https://www.youtube.com/shorts/rxUXB-EbOec

farfromrefug commented 2 years ago

@brianrclow i try to take a look at it. I hope it is not a native bug cause the lib is not maintained anymore :s

brianrclow commented 2 years ago

@farfromrefug I don't think it a native bug but I am not 100% sure. I imagine it's just a math error in calculating the number of ticks which effects where the initial value position is at.