Skip to content

Conversation

@bskimball
Copy link

I was getting an error saying the propTypes were undefined for Packery. I compared the code to the Masonry package which I did not get the errors. I just changed the module from a function to a React class and explicitly required React

I was getting an error saying the propTypes were undefined for Packery. I compared the code to the Masonry package which I did not get the errors. I just changed the module from a function to a React class and explicitly required React
@aknuds1
Copy link
Contributor

aknuds1 commented Jun 28, 2016

I don't know right off the bat why the Packery component is wrapped in a function that takes React as an argument, but I cannot see why you should get any errors if you call said function with the React module. It should be functionally equivalent to your fork.

@bskimball
Copy link
Author

bskimball commented Jun 28, 2016

I don't know why I was getting the error either. I am using es2015 import syntax to require the packery component along with React 15.1. I know the minor change fixed the issue.

@eiriklv
Copy link
Owner

eiriklv commented Jun 28, 2016

@bskimball @aknuds1
I haven't had time (yet) - but you could take a look at react-masonry-component and "port" everything from there to this component (there's a large amount of the improvements done there..). They are essentially doing the exact same thing - except one is using Masonry and the other one Packery. This one using Packery haven't gotten any TLC in a long time, but the Masonry one has (actively maintained by @afram).

@bskimball
Copy link
Author

I have altered the commit. The masonry component has been "ported" to the packery component.

@eiriklv
Copy link
Owner

eiriklv commented Jun 29, 2016

@bskimball Update the version to 2.0 (in package.json) as well and I'll merge it in + update the npm module. If you're interested in contributing to this then I will also gladly make you a contributor with push privileges! This goes for anyone interested :-)

@bskimball
Copy link
Author

The version has been updated to 2.0.0. Sure I would be happy to contribute where I can.

removed the React injection which is no longer necessary as of version 2. Added an es2015 example.
@lucbelliveau
Copy link
Collaborator

Just saw this PR... I submitted PR #18 to fix the same issue, but I didn't have time to refactor into a class... Ideally you would use the prop-types package, and declare it as a class... #18 only goes half-way and uses the create-react-class package as a quick fix.

@bskimball
Copy link
Author

@lucbelliveau my PR never got merged, but it does (did) work. It should be fairly straight forward to change it to extending the React Component. You could also move the extend function as a method in the class instead of keeping it separate. I'm not sure this is still maintained, so you might be better off forking this to your own branch.

@eiriklv
Copy link
Owner

eiriklv commented Oct 23, 2017

@bskimball @lucbelliveau I haven't been maintaining this thing for ages - any of you want push access?

@eiriklv
Copy link
Owner

eiriklv commented Oct 23, 2017

This whole project should be reconciled with react-masonry-component as the React wrapper around it would be nearly identical, and that project is maintained (by someone else than me).

@eiriklv
Copy link
Owner

eiriklv commented Oct 23, 2017

Added you both to the repo and npm ✌️

@lucbelliveau
Copy link
Collaborator

Thanks @eiriklv

I don't seem to have write access

@eiriklv
Copy link
Owner

eiriklv commented Oct 24, 2017 via email

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.

4 participants