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

Masonry support for collections #277

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

peiche
Copy link
Contributor

@peiche peiche commented Jul 28, 2015

Per the last point on the wish list, I've added a masonry option ("yes" or "no") to the Collection component screen and shortcode.

[aesop_collection title="Hello" collection="1" columns="3" splash="off" masonry="yes"]

masonry-screen

A couple things need some attention, though. Firstly, the description of the masonry option. I don't know what to put there, so you guys can worry about that. ;) Second, right now the masonry lib is always loaded. It should probably be conditional. (The ASE-specific script is always loaded too, but it's concatenated into ai-core.min.js. This will throw an error if the masonry lib isn't loaded, so I leave that decision to you guys.)

Cheers!

@michaelbeil
Copy link
Contributor

Thanks so much for the work that you have done. People like you help Open Source Projects to continue to thrive.

This could be a great addition, however, I think it may need some refactoring before it could be merged, such as the aforementioned conditional loading of masonry.js. Also, the component CSS files have all been changed. What did you do there? It just appears like they aren't minified.

@peiche
Copy link
Contributor Author

peiche commented Jul 28, 2015

I don't think they were changed, save for collection.less. I think they're just what resulted from running grunt less.

I agree that some refactoring needs to happen, but I wanted to at least get some of the larger pieces in place.

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

Successfully merging this pull request may close these issues.

2 participants