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

_calcItemPositions should use Math.floor() instead of Math.round() #33

Closed
slavap opened this issue Aug 9, 2016 · 9 comments
Closed

Comments

@slavap
Copy link

slavap commented Aug 9, 2016

cols = Math.round(self.width() / self.find('.filtr-item').outerWidth());

For example if container is 1024px, and tiles are 400px, cols will be calculated as 3, and as result the last tile in row will be only partially visible. In many cases this behavior is not desired, and can be fixed by using Math.floor(). So ideally there should be an option to control this behavior.

@giotiskl
Copy link
Owner

giotiskl commented Aug 9, 2016

@slavap this is being fixed by a pull request i will soon merge into the project and will be in the next patch which is shortly coming, so hold tight

@slavap
Copy link
Author

slavap commented Aug 9, 2016

Sorry, I see PR for fixing this. But IMO it should be an option, in some cases it could be ok to show last tile in row partially (not by default of course).

@giotiskl
Copy link
Owner

giotiskl commented Aug 9, 2016

I might consider adjusting the PR to make this optional, I'll give it some though during the weekend

@slavap
Copy link
Author

slavap commented Aug 9, 2016

Thanks! I really like your component.

@slavap
Copy link
Author

slavap commented Mar 16, 2018

@giotiskl Still not fixed :-( Have you decided to leave this problem "as is" ?

@giotiskl
Copy link
Owner

@slavap this will be made an option as suggested by you, the initial idea was that relative percentage widths should be used for the grid amounting up to 100%. Nevertheless, it is something that I could now work on given that the major rewrite of the core was finished in 1.3.0 which was of higher importance.

@slavap
Copy link
Author

slavap commented Mar 16, 2018

@giotiskl Thanks! It would be really nice to have it fixed somehow.

@slavap
Copy link
Author

slavap commented Jun 4, 2019

@giotiskl Found one more problem in FilterContainer.calcColumns(). It should be protected against NaN case, because sometimes when filterizr is not visible on screen, but browser window is resized, I'm getting error "RangeError: Invalid array length". To fix it calcColumns() has to be like this:

calcColumns() {
    var v = Math.round(this.props.w / this.props.FilterItems[0].props.w); // Math.floor() should be here as well!
    return isNaN(v) ? 0 : v;
} 

@giotiskl
Copy link
Owner

giotiskl commented Jul 9, 2019

Post v2.2.0 the logic has been altered to use Math.floor.

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

No branches or pull requests

2 participants