Skip to content

MudSlider: Fix tick count calculation for certain cases. Fixes #8713#8730

Merged
henon merged 1 commit intoMudBlazor:devfrom
digitaldirk:patch-1
Apr 18, 2024
Merged

MudSlider: Fix tick count calculation for certain cases. Fixes #8713#8730
henon merged 1 commit intoMudBlazor:devfrom
digitaldirk:patch-1

Conversation

@digitaldirk
Copy link
Member

@digitaldirk digitaldirk commented Apr 17, 2024

Description

Used ToDecimal instead of ToDouble because for certain values the double/int math would have rounding errors (see related issue for details). Fixes #8713

How Has This Been Tested?

Visually tested on docs.

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Before:
Screenshot_20240415_042123

After:
Screenshot_20240415_042240

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@digitaldirk
Update tick count calculation to fix certain cases. Fixes MudBlazor#8713
76b1bba 


Used ToDecimal instead of ToDouble because for certain values the double/int math would have rounding errors. Fixes MudBlazor#8713
@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Apr 17, 2024
@ScarletKuro ScarletKuro requested a review from henon April 17, 2024 22:04
@codecov
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.07%. Comparing base (28bc599) to head (76b1bba).
Report is 67 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #8730      +/-   ##
==========================================
+ Coverage   89.82%   90.07%   +0.24%     
==========================================
  Files         412      418       +6     
  Lines       11878    12050     +172     
  Branches     2364     2366       +2     
==========================================
+ Hits        10670    10854     +184     
+ Misses        681      661      -20     
- Partials      527      535       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ScarletKuro
Copy link
Member

ScarletKuro commented Apr 17, 2024

Hi.
Thanks for the PR.
CalculatePosition method doesn't require the same improvement?

@digitaldirk
Copy link
Member Author

digitaldirk commented Apr 17, 2024

Hi.
Thanks for the PR.
CalculatePosition method doesn't require the same improvement?

I do not believe so. That just calculates the width of the component. I just tried a similar fix to the CalculatePosition method on the docs and the width of the sliders remained the same px as before.

In my testing for this pr I had a value like 2.999999999987 and it went to 2 because of the int. But in this case 2.999999999987 would be rounded to 3 and not truncated to 2.

@henon henon merged commit 8259375 into MudBlazor:dev Apr 18, 2024
@henon
Copy link
Contributor

henon commented Apr 18, 2024

Thanks!

@digitaldirk digitaldirk deleted the patch-1 branch April 18, 2024 14:32
biegehydra pushed a commit to biegehydra/MudBlazor that referenced this pull request Apr 26, 2024
@digitaldirk @biegehydra
MudSlider: Fix tick count calculation for certain cases. Fixes MudBla…
45cb167 
…zor#8713 (MudBlazor#8730)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ScarletKuro ScarletKuro ScarletKuro approved these changes

@henon henon Awaiting requested review from henon

Assignees

No one assigned

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Slider: Tick count math causes incorrect number of ticks for certain values

3 participants

@digitaldirk @ScarletKuro @henon