bigbite / macy.js

http://macyjs.com/
MIT License
1.3k stars 156 forks source link

macy.recalculate doesn't calculate top margin for first row #48

Closed ProQuestionAsker closed 5 years ago

ProQuestionAsker commented 5 years ago

Hey there! First of all, I'm loving Macy.js so thank you for this!

Second, I'm working on adding a filter feature to my layout. The initial macy instance loads great and everything appears in the correct place. For reference, I have the margin: 24 in the initial macy instance. The margin of 24 px seems to apply a uniform margin around all sides of all items except the top margin of items on the top row (which have a 0px margin). This is all fine (see the image below)

screen shot 2018-11-04 at 6 45 11 pm

In order to filter, I'm essentially just alternating the display value for each item between none (to hide it) and flex to show it. After doing that, I run macy.recalculate(true) which does rearrange the objects, but it doesn't seem to take the top margin into account. As I mentioned above, during the initial arrangement of the tiles, items in the top row have a top: 0px value whereas items that moved up to the top row after the recalculation now have a top: 24px value.

screen shot 2018-11-04 at 6 45 36 pm

It seems like the top margin comes in when items started in lower rows and are moved to the top row during a recalculate() call?

Also, for reference, I tried macy.reInit() instead of macy.recalculate() and I'm finding the same issue.

I've been messing with this for the past few hours and haven't been able to find a way to fix this. Any input would be much appreciated. Thank you!!

jrmd commented 5 years ago

Hi @ProQuestionAsker

Sorry to see you're having issues.

As a suggestion have you tried adding either margin-top the macy-container rather than trying to add it to the elements inside, or adding margin-bottom to the filters container. This make it much easier for you to control the margins.

As another method, you could set the vertical margins to 0 and wrap each item in the macy object in a container, and add padding top so they always have the spacing.

It is hard for me to come up with the best solution without viewing your code.

Let me know if you get it working with one of these solutions.

Thanks, Jerome

ProQuestionAsker commented 5 years ago

No worries at all, thanks for the quick response and assistance @jrmd!

I've added a version of the site here: https://elastic-fermat-c4dfa4.netlify.com/

I've tried what you have suggested but none of them seem to work. They all do add some padding or space, but they still prevent the top row from aligning after a macy.recalculate() call.

It ultimately seems to be an issue with the way that macy is calculating the margin on the items (specifically for those in the top row in comparison to those in lower rows).

jrmd commented 5 years ago

Hi @ProQuestionAsker

After looking at what your example, i see what you mean, it seems like i dont take into account that some items aren't visible so i continue to place them on the grid.

I will look into fixing this soon, however as a solution in the mean time you could remove items from the grid then recalculate. Although this isn't ideal, I'll look into this as soon as i have chance.

Sorry for any inconvenience.

Thanks, Jerome

jrmd commented 5 years ago

Hi @ProQuestionAsker

This issue should now be resolved and has been released as version 2.3.2 on npm

Once upgraded if the issue still occurs feel free to re-open the ticket.

Thanks, Jerome

ProQuestionAsker commented 5 years ago

Works like a charm! Thank you!!