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

Bug in util.js causes "Error: lockOffset value should be a finite. Given NaN" when lockToContainerEdges is used #197

Open
cbcf opened this issue Nov 5, 2023 · 1 comment · May be fixed by #198

Comments

@cbcf
Copy link

cbcf commented Nov 5, 2023

Symptom:
When using lockToContainerEdges on the SlickList (or ContainerMixin), the library throws an error:
Error: lockOffset value should be a finite. Given NaN
This happens without setting lockOffset or when setting it to a single pixel or percent value.

Cause:
The getLockPixelOffset function receives a NaN due to a bug in the getLockPixelOffsets function.
Any string for lockOffset is always forcedly converted to a number (using the unary + operator):

if (typeof lockOffset == 'string') {
    lockOffset = +lockOffset;
}

If the string is not a pure number, it is converted to NaN. The result is passed on to the getLockPixelOffset function, which in turn has actual code to handle strings with a px or % suffix. Accordingly, as the default value used for lockOffset is "50%", this also converts to NaN and causes the error.

Suggested fix:
Remove the invalid and redundant string-to-number conversion shown above and also correct the associated type annotations. I will add a PR addressing this.

Workaround for now:
Explicitly set lockOffset to an array of strings when using lockToContainerEdges.
For example, to get the expected behavior of the default value of 50% for lockOffset, explitcitly set it to `['50%', '50%'], e.g.:

<SlickList lock-to-container-edges :lock-offset="['50%', '50%']" ...>
@cbcf
Copy link
Author

cbcf commented Nov 5, 2023

Here is a JSFiddle showing the Error: https://jsfiddle.net/q351876g/43/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant