Skip to content

Conversation

@frostbolt
Copy link

@frostbolt frostbolt commented Apr 11, 2019

Hi.

Your plugin depends on outdated jQuery version, that is no longer maintained
At first I was going to move it to third jQuery but then I noticed that there's only one jQuery call that can be easily replaced with native code.

As you can see here, native .map realization is compatible with all modern (IE9+) browsers.

@atais
Copy link
Owner

atais commented Apr 11, 2019

dude the eonasdan-datetimepicker uses jQuery.
this is just a wrapper.

you will need jQuery anyway to run the library.

@atais atais closed this Apr 11, 2019
@frostbolt
Copy link
Author

yeah, but it uses jquery itself.
your implementation requires an outdated 2nd version, which has security vulnerabilities, according to npm audit.

It's required to get rid of it's usage or update the dependency.

@atais atais reopened this Apr 11, 2019
@atais
Copy link
Owner

atais commented Apr 14, 2019

Sorry I did not have time to merge it
however, i do not know why but travis is not showing up here.
But your changes fail on tests:
https://travis-ci.org/atais/angular-eonasdan-datetimepicker

if you fix it I can merge the change.
I think fixing the dependency is easier

@atais
Copy link
Owner

atais commented Apr 24, 2019

@frostbolt now travis works

@frostbolt
Copy link
Author

@atais I'll take a look later this month.

@frostbolt
Copy link
Author

frostbolt commented Apr 30, 2019

Hi, @atais,
Actually I found an issue in my code 🌚 , but tests still doesn't work.

When I try to run them it seems like angularjs doesn't work. I can see {{expressions}} inside the page.

But the most strange thing is that I've managed to run them manually running npm start. Looks like everything is just fine.
image

PS. I've tried to run the tests for origin/master. Unsuccessfully.

@atais
Copy link
Owner

atais commented May 1, 2019

Damn it happens from time to time because I don't update the dependencies and the tests have to be fixed.
I am currently on holidays but I will try to fix it once I am back

@frostbolt
Copy link
Author

I noticed that I've accidentally moved several dependencies from dev to common section. Fixed that.

I've also replaced ' with \" as long as 1st variant causing error on windows platform (see screenshot below).
image

@atais
Copy link
Owner

atais commented May 21, 2019

I have just looked into it, but it does not seem easy.
I do not know why it stopped working and what is the cause.

To be honest, I am not working with those technologies anymore so I really do not want to step into it.

If you want and could fix the tests I will gladly merge the changes.

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.

2 participants