fix(perf): only compute snappoints when step is provided #2699
Conversation
This keeps the snapping as it snaps points to the rounded number if no snappoints are provided. Until rheostat provides a lazy way to fix the snappoints, there will also be a risk of having an "out of memory" error because the range of values is too big. There is an open issue opened for adding this new API: airbnb/rheostat#14 Fix #2662
|
Deploy preview for algolia-instantsearch ready! Built with commit c511df2 https://deploy-preview-2699--algolia-instantsearch.netlify.com |
|
Deploy preview for algolia-instantsearch ready! Built with commit a821f78 https://deploy-preview-2699--algolia-instantsearch.netlify.com |
src/components/Slider/Slider.js
Outdated
| @@ -55,6 +55,7 @@ export class RawSlider extends Component { | |||
|
|
|||
| // creates an array of values where the slider should snap to | |||
| computeSnapPoints({ min, max, step }) { | |||
| if(!step) return undefined; | |||
There was a problem hiding this comment.
I think this is not valid to our linter, might do:
if (step) {
return [...range(min, max, step), max];
}
|
Impressive debugging @bobylito you nailed it 💯 |
There was a problem hiding this comment.
This is great, and a right way to go for it.
A good workaround for now, but I wonder if an option would be to implement airbnb/rheostat#14 🤔
Anyway, according to travis you still need to update the snapshots
done
Yes. That could be the end game. Let's move forward with this one. I dismiss your review @Haroenv ;) |
bobylito
dismissed
Haroenv’s stale review
Taken into account. The fix in rheostat is a longer term solution.
This keeps the snapping but does not generate the snap points if no value is provided. We can do that because rheostat snaps the point to the rounded value by default (if
snap=true). Until rheostat provides a lazy way to fix the snappoints, there will be a risk of having an "out of memory" error because the range of values can be too big. There is an open issue opened for adding this new API: airbnb/rheostat#14 but it's uncertain if someone will work on it.Fix #2662