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

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

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

digitaldirk
Copy link
Contributor

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.



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 Something does not work as intended/expected PR: needs review labels Apr 17, 2024
@ScarletKuro ScarletKuro requested a review from henon April 17, 2024 22:04
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

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

@digitaldirk
Copy link
Contributor Author

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
5 checks passed
@henon
Copy link
Collaborator

henon commented Apr 18, 2024

Thanks!

@digitaldirk digitaldirk deleted the patch-1 branch April 18, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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