search

angular-ui / ui-calendar

A complete AngularJS directive for the Arshaw FullCalendar.
http://angular-ui.github.io/ui-calendar/
MIT License
1.49k stars 729 forks source link

Edit initCalendar to work properly with jQuery #402

Closed LukeSchlangen closed 8 years ago

LukeSchlangen commented 8 years ago

Resolve #267 "calendar.fullCalendar is not a function" This is simply the change mentioned in the comments on that issue. I haven't seen anything stating that this breaks the build for others or has a negative consequence, so I think this would be a good change to fix it so others do not have this issue going forward.

martin-langhoff commented 8 years ago

Sounds like a good change, reducing friction for those wanting to use ui-calendar this way.

Need to ask for some further effort,

LukeSchlangen commented 8 years ago

Thanks Martin.

This is my first pull request for ui-calendar, so I'm new to this.

Thanks for the quick response!

martin-langhoff commented 8 years ago

All good questions!

neeckeloo commented 8 years ago

👍

shenqihui commented 8 years ago

+1

martin-langhoff commented 8 years ago

@LukeSchlangen -- master has now been updated, can you rebase? The automated tests are fixed, so if your rebased branch is clean it'll be good. Apologies for the delay.

martin-langhoff commented 8 years ago

Your rebase isn't clean, you've ended up with conflict markers in an intermediate commit. Think you can clean up your branch to contain just one commit with the desired change? AFAICS if you just do that the CI tests should work. Thanks.

martin-langhoff commented 8 years ago

Ah, you fixed it up, so the branch is now good, however there's the intermediate broken commit. Can you fix up the branch (via a rebase interactive for example)?

LukeSchlangen commented 8 years ago

Oh no, you're watching this. Embarassing 😄

I've actually never rebased before, so I'm learning as I go, so this might be a little ugly. I'll see what I can do to clean this up.

LukeSchlangen commented 8 years ago

So I think I cleaned it up? The one thing that seems off is that it says I made a change to the last line by not having a new line at the end of the file, but I'm looking at the current code, I don't see that there either? Should I add an empty line at the end of my file and try rebasing one last time?

martin-langhoff commented 8 years ago

Looks like you've cleaned it up already. That's good.

CI is failing with a minor error re that missing last newline. Trivial fix :-)

LukeSchlangen commented 8 years ago

Do you want me to add it? I think I've got the idea now.

ovaar commented 7 years ago

This fix is not available in bower yet. Is there a way to fix this?

LukeSchlangen commented 7 years ago

It wasn't available in npm the last time I tried either. I'm not sure how to update those? I'm using my own local branch still for this reason (the branch merged here), is there something I can do to update npm and bower or does it need to be an owner?

ovaar commented 7 years ago

You should commit the changes to github, because if you look at https://github.com/angular-ui/ui-calendar/blob/master/src/calendar.js#L284 it still says calendar = angular.element(elm).html('');

LukeSchlangen commented 7 years ago

I think I committed them to my branch and the branch I had control over? I don't have access to this repo at this time, so I'm not sure if I can do it from here. It's a quick change so I could do it again, but I guess I'm not sure why it isn't working? I'm not the owner of angular-ui/ui-calendar, so I don't think I can control the file you mentioned? maybe @martin-langhoff can help us with how to proceed?

marc1404 commented 7 years ago

Any updates on this issue? Will your change @LukeSchlangen be published to npm?

LukeSchlangen commented 7 years ago

I'm not sure how to do this. I don't have access to that repo at this time (I stopped working for the organization where that branch exists). The change is small and I can recreate the change and make another pull request. I'm just not sure how this works. If there are instructions on how to do this or what I am missing, I'd be open to trying to fix or redo. I'm wondering if @martin-langhoff could provide some direction?

marc1404 commented 7 years ago

It seems like your pull request has been reverted in this commit 568e4311206de68beeac5cc2d47a9241a8247f78 While I agree that angular-ui-calendar should not dictate the usage of jQuery, your fix is still needed in my opinion

LukeSchlangen commented 7 years ago

I supppose @peterver @joshkurz or someone else would need to make that decision.

ovaar commented 7 years ago

+1