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

Ceil function is now used when calculating the item size #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ceil function is now used when calculating the item size #282

wants to merge 1 commit into from

Conversation

Morerice
Copy link

@Morerice Morerice commented Apr 8, 2017

This pull request is related to this issue: #200

The problem occurs because the round function is used to calculate the item width. Hence, if the width of an item is 10.3px it will be rounded to 10 and therefore, the calculated width of the frame won't be enough to contain the items.

This is shown here: https://jsfiddle.net/0cvejr3o/

In my pull request, I simply used the ceil function to calculate the width of each item.

@gigatyrant
Copy link

Perfect, thank you ! You saved my day.

Merge this fix please :-)

@kleinfreund
Copy link

kleinfreund commented Aug 16, 2017

I was looking into this issue and tried your modifications. They resolve the issue of not leaving enough space for the last item in my scenario, however, now there is too much space left.

Either way, using round or ceil when calculating the item size will generally lead to a wrong total size when multiplying item size by number of items.

Instead, the the natural item size needs to be used to calculate the total size. Maybe then it’s helpful to ceil this result to the next integer value.

Update: I get pretty good results when just leaving out round/ceil in the calculation of the item size. The calculated total width generally is a decimal value then, but for now it seems fine.

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 this pull request may close these issues.

3 participants