Added support to jQuery UI Datepicker for AngularJS

vitalets / angular-xeditable

Edit in place for AngularJS

http://vitalets.github.io/angular-xeditable

MIT License

1.91k stars 403 forks source link

Added support to jQuery UI Datepicker for AngularJS #592

Closed arielcr closed 7 years ago

arielcr commented 7 years ago

Implementation of editableUiDate directive to be able to use jQuery UI datepicker for AngularJS

To use the new directive you simple do this:

<span editable-uidate="yourModel" e-ui-date="datePickerConfig">{{ yourModel }}</span>

And use it as you normally will use the jQuery UI datepicker.

ckosloski commented 7 years ago

Hi @arielcr, thank you for the pull request. Please remove the generated files from your request. Add a demo/test of your new directive in docs/demos (will also require changes to scripts-dev.jade and structure.js) Make sure all existing tests still run.

arielcr commented 7 years ago

Hi @ckosloski , thanks for your reply!

So, I must not grunt build anything? Just push the changes on /src?

ckosloski commented 7 years ago

You can grunt build for testing locally, but only push the src changes.

arielcr commented 7 years ago

@ckosloski Is it ok like this?

ckosloski commented 7 years ago

Yes, better, now please add documentation/tests in docs/demos.

arielcr commented 7 years ago

@ckosloski ready, can you take a look at it? thanks!

ckosloski commented 7 years ago

Almost there :)

I see that you upgraded the version of jQuery, in your documentation can you include the minimum version of jQuery to use for this component?
Remove the fiddle from your doc link since it's just pointing to the angular-ui-bootstrap date picker and not yours
Just to verify, all the tests pass, correct?

Thanks!!

arielcr commented 7 years ago

@ckosloski Yep, all tests are passing!

ckosloski commented 7 years ago

OK, this looks good to me. Time for @eugef to take a look and merge

eugef commented 7 years ago

Hi @arielcr, the way how we include 3rd party dependencies right now is completely wrong (i mean copying them to the libs folder), the right way would be to use bower to install them.

Please do the following:

remove jquery-ui and angular-ui-date from libs
install them as devDependencies of bower
change urls to include jquery-ui and angular-ui-date directly from bower_components folder

I did that already for angular and jquery (https://github.com/vitalets/angular-xeditable/commit/e4f3e25b93219dd7431329941f860e9d5b7e2a76)

I've already modified the package.json so whenever you run npm install bower componets are also updated.

arielcr commented 7 years ago

Hi @eugef !

I did the changes that you mentioned. It seems that there's a conflict on bower.json (because of the two additional dependencies that I added). I updated mine with the latest one from your repo and added the two dependencies, but I think that you must be the one who merge them.

Let me know if there's anything else to be done for this pull request.

It's a pleasure to contribute to this awesome library!

eugef commented 7 years ago

@arielcr , thanks for the quick fix. Please also squash all you commits into the single one.

arielcr commented 7 years ago

@eugef Ready, commits squashed!

eugef commented 7 years ago

@arielcr, it seems like you squashed wrong commits, please make sure you don't add anything to libs folder, install all dependencies with bower instead.

arielcr commented 7 years ago

@eugef You're right. My mistake! I've just fixed it. Can you check it out?

eugef commented 7 years ago

Now it's good. Thanks for your PR.