search

eiriklv / react-packery-component

A React.js component for using @desandro's Packery
MIT License
43 stars 35 forks source link

change from a function to a React class #9

Open bskimball opened 8 years ago

bskimball commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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

eiriklv commented 8 years ago

@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 commented 8 years ago

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

lucbelliveau commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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

eiriklv commented 7 years ago

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 commented 7 years ago

Added you both to the repo and npm ✌️

lucbelliveau commented 7 years ago

Thanks @eiriklv

I don't seem to have write access

eiriklv commented 7 years ago

https://github.com/eiriklv/react-packery-component/invitations

  1. okt. 2017 kl. 14.10 skrev Luc Belliveau notifications@github.com:

Thanks @eiriklv https://github.com/eiriklv I don't seem to have write access

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eiriklv/react-packery-component/pull/9#issuecomment-338968940, or mute the thread https://github.com/notifications/unsubscribe-auth/ADUybQ8e1MQlU2KMXsoswtJ4Pcphuirmks5svdOzgaJpZM4JAbPA.