Updating dependencies, updating rollup config file

[LostInBrittany]

Hi! Thanks a lot for having updated this module to D3 version 4+!

I am using it in a D3 v5 project (it works great!), so after having tested it, you have here a PR updating the dependencies (normal, peer and dev). As recent Rollup versions have changed the format of the config file, I've also updated it.

Thanks again for the good job!

[denisemauldin]

Hi @LostInBrittany !

Thanks for the PR. I think the package-lock.json gets generated by npm v5 right? I know there's some debate on whether or not to commit it to source control. What do you think are the advantages of including it comparing to having npm regenerate it?

[LostInBrittany]

Well, I prefer to commit the package-lock.json, as it's a kind of guarantee of working condition for the dependencies.

I mean, for me, in package.json we are kinda optimistic, we say it should work with dependency xyz version a.b.c or greater, and it should work. But we are only sure that it works on version a.b.c. Commiting the package-lock.json is for me a failsafe way to allow people to test and use the lib without having to deal with any possible dependency problem.

But that's only my opinion, of course ! :) :)

On Thu, Apr 26, 2018 at 10:19 PM, Denise Mauldin notifications@github.com wrote:

Hi @LostInBrittany https://github.com/LostInBrittany !

Thanks for the PR. I think the package-lock.json gets generated by npm v5 right? I know there's some debate on whether or not to commit it to source control. What do you think are the advantages of including it comparing to having npm regenerate it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/denisemauldin/d3-timeline/pull/13#issuecomment-384776066, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsVzLgDZX_x9RRRy5ZWzp4J52zH2SK5ks5tsivGgaJpZM4TlQQL .

[LostInBrittany]

Hi! I've updated the PR as I have changed the default for colors() to d3.scaleOrdinal(d3.schemeCategory10) instead of d3.scaleOrdinal(d3.schemeCategory20).

The reason is that d3.schemeCategory20 has been removed from d3 v5.

[LostInBrittany]

Hi!

There are some errors I need to check before being able to submit you the PR in full confidence, I close it by now and I will reopen it as soon as I will fix them.

[LostInBrittany]

Ok, I made all the examples work :)

I had a problem with allowZoom. As written, it only allows zoom is allowZoom(false) is configured. As I understand it should the the oposite, I have change it. If I am wrong, sorry about that!

[LostInBrittany]

Hello!

When do you think you will have a moment to look at this PR?

Sorry to insist, I am using it in an app, and I would prefer dropping the dependency to my fork and put this one :)

[denisemauldin]

@LostInBrittany Ah, I didn't realize you were done trying to fix it. :) Everything is working now?

[LostInBrittany]

Oups, sorry, I guess I didn't communicate well, sorry again :(

Yes, everything is working for me, I'am already using it in a web component ( https://www.webcomponents.org/element/LostInBrittany/granite-timeline ) that I'm using in several internal apps right now :)

On Mon, May 21, 2018 at 7:13 PM, Denise Mauldin notifications@github.com wrote:

@LostInBrittany https://github.com/LostInBrittany Ah, I didn't realize you were done trying to fix it. :) Everything is working now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/denisemauldin/d3-timeline/pull/13#issuecomment-390720028, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsVzF7inxTMrPuWkL7M29eAJ1IfQgfGks5t0vWzgaJpZM4TlQQL .

[denisemauldin]

@LostInBrittany This PR results in a change in functionality. Please address comment.