fix(Pit): round the slider pit value #2904
Conversation
|
Deploy preview for algolia-instantsearch ready! Built with commit f562f37 https://deploy-preview-2904--algolia-instantsearch.netlify.com |
|
The change itself is fine I think, but it should probably be done behind an option |
|
Hi @Haroenv, it's better I think |
There was a problem hiding this comment.
From reading the code it seems that there is an intent for rounding but because of the parenthesis it came out as not working. This is a bug fix IMHO.
Thanks a lot @rezak-otmani for making this project better :)
|
Actually the problems lies in prettier (it's actually the check that fails) and parenthesis seems very troublesome as a problem (prettier/prettier#187). @rezak-otmani can you find a way to rewrite that code so that it doesn't rely on parenthesis (like introducing an intermediate variable that holds the result of parseFloat)? |
* refactor(connectInfiniteHits): use jest instead of sinon in tests * test(connectInfiniteHits): confirm algolia#2928 * test(connectInfiniteHits): remove unsupported option from test * fix(connectInfiniteHits): fix algolia#2928 * refactor(connectInfiniteHits-test): review from @samouss
|
Prettier 1.13 might fix the calculation issue: Insert more parentheses in math expressions. |
tkrugg
force-pushed
the
develop
branch
2 times, most recently
from
2b99b50
to
6006fe1
Compare
francoischalifour
force-pushed
the
develop
branch
from
6006fe1
to
6a58b99
Compare
francoischalifour
changed the base branch from
develop
to
fix/slider-round-pit-value
| @@ -11,7 +11,7 @@ const Pit = ({ style, children }) => { | |||
| // Children could be an array, unwrap the value if it's the case | |||
| // see: https://github.com/developit/preact-compat/issues/436 | |||
| const value = Array.isArray(children) ? children[0] : children; | |||
| const pitValue = Math.round(parseFloat(value) * 100) / 100; | |||
| const pitValue = Math.round(parseInt(value) * 100) / 100; | |||
There was a problem hiding this comment.
| const pitValue = Math.round(parseInt(value) * 100) / 100; | |
| const pitValue = Math.round(parseInt(value, 10) * 100) / 100; |
francoischalifour
changed the base branch from
fix/slider-round-pit-value
to
develop
francoischalifour
changed the base branch from
develop
to
fix/slider-round-pit-value
|
I changed the base branch so that we can apply changes to your fix. Thank you @rezak-otmani! |
francoischalifour
changed the title
* fix(Pit): round the slider pit value (#2904) * fix(Slider): use parseInt base
# [3.5.0](v3.4.0...v3.5.0) (2019-05-14) ### Bug Fixes * **hitsPerPage:** improve warning for missing state value ([#3707](#3707)) ([93d8432](93d8432)) * **numericMenu:** prevent refinement reset on checked radio click ([#3749](#3749)) ([e4a6e75](e4a6e75)) * **rangeSlider:** round the slider pit value ([#3758](#3758)) ([6edee3e](6edee3e)), closes [#2904](#2904) * **types:** improve UiState types ([#3763](#3763)) ([e8ea57b](e8ea57b)) * **voice:** import correct noop ([#3766](#3766)) ([6a80422](6a80422)) ### Features * **voiceSearch:** add connector and widget ([#3601](#3601)) ([21e4d81](21e4d81))
resolves #2828
always rounding the middle value to a whole number
Summary
Result