Skip to content
This repository has been archived by the owner on Dec 30, 2018. It is now read-only.

Work without needing jQuery #40

Closed
dremonkey opened this issue Jan 24, 2014 · 19 comments
Closed

Work without needing jQuery #40

dremonkey opened this issue Jan 24, 2014 · 19 comments

Comments

@dremonkey
Copy link
Contributor

Curious as to what your thoughts are on making this work without needing jQuery. I have a working fork if you would like to take a look.

https://github.com/dremonkey/angular-masonry/compare/nojquery

@passy
Copy link
Owner

passy commented Jan 24, 2014

I like it! That actually looks a lot cleaner. Would you mind opening a pull request for that with just the uncompiled source changed?

@dremonkey
Copy link
Contributor Author

No problem. Glad you like it. Just opened the pull request - #41

@macneib
Copy link

macneib commented Oct 23, 2014

Will this PR be accepted? I would like to use this module without jQuery.

@passy
Copy link
Owner

passy commented Oct 23, 2014

@macneib Totally. If you want to pick up the previous work and clean it up, I'm more than happy to merge it.

@nonec
Copy link

nonec commented Oct 29, 2014

I've added the changes to your current version and it works for me (didn't work at all before) :)

@macneib
Copy link

macneib commented Nov 2, 2014

@nonec it would be awesome if you would be willing to submit those changes.
I'm debating the idea of working on this, but your work would save me a lot of time.
Cheers!

@nonec
Copy link

nonec commented Nov 2, 2014

Sure, I'd like to help! But I didn't include all the changes. Only the ones made to "angular-masonry.js" because I downloaded this as standalone. Will it still be of help?

@macneib
Copy link

macneib commented Nov 3, 2014

Would you be willing to fork this repo, insert your changes to angular-masonry and apply your changes with a PR? I would be more than happy to do that for you but you'll need to put the files up somewhere.

@macneib
Copy link

macneib commented Nov 3, 2014

Ok, I've spent some time looking over the fork by @dremonkey, and it seems solid.
The primary barrier with any PR rests on the actions on a brave soul who is wililng to adapt the test scripts.
I don't have the knowedge required to port the jquery elements at this time, however I'll tinker away at it.
Until then I'll be using the forked version.
Learning jquery so I can gid rid of it. sigh...

@nonec
Copy link

nonec commented Nov 3, 2014

Yeah it is. I didn't have to change much. It just didn't work out of the box. I have set up the fork, but it's really just a hack of the changes made by @dremonkey put into the current codebase. So I'm not sure if a PR will be useful for this. If you want to work an that to prepare the PR let me knew and I can add you to that fork.

@macneib
Copy link

macneib commented Nov 3, 2014

@dremonkey's code worked fine for me. Do you have a nojquery branch on your fork?

@nonec
Copy link

nonec commented Nov 3, 2014

Nope, just put the changed .js-file in there. But there could be one created...

@macneib
Copy link

macneib commented Nov 3, 2014

this part. This is hard for me to understand.

$.fn.masonry = sinon.spy()

@nonec
Copy link

nonec commented Nov 3, 2014

I guess it's using the sinon framework for the tests to spy on the masonry:
http://sinonjs.org/

Here is an explanition of the difference between sying and mocking:
http://stackoverflow.com/questions/12827580/mocking-vs-spying-in-mocking-frameworks

@macneib
Copy link

macneib commented Nov 3, 2014

@passy just want to check what the house rules are before I get too involved.

I'd like to structure the test like so /src/angular-masonry.spec.js

Since I'm at it, would there be any objection to updating the Gruntfile to use ngAnnotate instead of ngMin which is obsolete atm?

There are some other things but I expect with my schedule being the way it is modifiying everything will take some time.

@TimPetricola
Copy link

Any news on this one?

@macneib
Copy link

macneib commented Feb 26, 2015

Day job called. Pretty sure I won't have time for a while.

@simonsayscode
Copy link

Is there any progress on this? It seems like it's a pretty innocuous change since you don't need to depend on jQuery to utilize Masonry directly.

@passy
Copy link
Owner

passy commented Sep 22, 2015

Still happy to take PRs, @simonsayscode. :)

@jodytate jodytate mentioned this issue Feb 8, 2016
@passy passy closed this as completed in 19aca3b Jan 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants