Svg.js: 2.5.0 release

[dotnetCarpenter]

Things that needs to be visited before we release 2.5.0

• [x] Documentation for #582
• [x] Documentation for #590
• [x] ~Documentation for make the method plot a getter when no parameter is passed for SVG.Polyline,SVG.Polygon, SVG.Line, SVG.Path (related #547)~
• [x] Documention for allow SVG.PointArray to be passed flat array
• [x] ~Documentation for allow plot to be called with 4 parameters when animating an SVG.Line~
• [x] ~Documentation for relative value for SVG.Number are now calculated in its morph method (related #547)~
• [x] ~Fix TBox #594~
• [x] Add unit tests #593
• [x] [Create an issue with jsfiddle](https://github.com/jsfiddle/jsfiddle-issues/issues) to update to SVG.js 2.5.0
• [x] Decide if and how bbox() can be augmented with a method to get the transformed vectors (see #594 for motivation)

[Fuzzyma]

The relative values for svg number is an fx internal. There is no need to mention that in the docs except if you want to make relative numbers a feature (I do not suggest that ^^)

[wout]

Not sure about the relative value for SVG.Number, have to look into that.

Agree with all but plot() as a getter. The getter persona of the plot() method, to be honest, it feels a bit awkward. Yet another method to get the array while we already have the array() method? That's why I created array() in the first place. If you want the array of an object, do you call plot()? Or do you call array()?

For 3.0.0 this should be revised. In this case, if we keep plot() as the getter, the array() method should be removed altogether. I don't like having multiple methods returning the same value if they do not both serve a specific purpose.

I agree it is in line with the attr() method, though. One ruling them all, that's a good thing. Keeps things simple. It's something to think about going forward. I would like to keep the API lean. Having methods lying around that serve the same purpose as another one, that's a red flag.

The same can be said for the track() method on SVG.TextPath. Maybe it should be called path() (without an argument)? That makes sense...

UPDATE
Maybe plot() should be deprecated in favour of array() in 3.0.0? Just a thought.

[Fuzzyma]

This change was done to simplify to initAnimation method of the fx module. There was always one check needed if plot is there so we could get the array with array. It looks much cleaner now. However I agree, that methods with the same functionality are bad code smell. So I agree on your UPDATE

The relative Numbers are exclusively used in the fx module to make dmove, dx... animatable. It is not used anywhere else and isnt a function user would normally need. So I wouldnt mention it in the docs

[wout]

Maybe it's better not to document plot() as getter, for the time being. Just until we decide on the future API.

[wout]

Another thought. SVG.PointArray now accepts a flat array. How about SVG.PathArray?

[Fuzzyma]

It already does. See this lines:

      array = array.reduce(function(prev, curr){
        return [].concat.apply(prev, curr)
      }, [])

All arrays are normalized to flat arrays before beeing parsed to a patharray

[wout]

Ok, couldn't find it at first sight. Then I'll document that as well.

[wout]

There is not really a place for this one:

Documentation for allow plot to be called with 4 parameters when animating an SVG.Line

The corresponding methods on animate() should work exactly the same as the non-animated version. So I think we can skip this one.

[RmiTtro]

Maybe we could mention that plot can be animated in SVG.Line as we do it for SVG.Polyline, SVG.Polygon, etc...

[wout]

Good one, I'll include it the next time I update the docs.

It might be a good idea to mention at method level if they are animateable. Something like:

[RmiTtro]

I think that would be a good idea and since animated method are made to be used the same way as their non-animated counter-part that might be all that is needed to mention.

[wout]

Great, I'll do that then.

What would be better? Mentioning animateable or the one below?

[RmiTtro]

Both are fine but I think I will cast my vote on the second one.

[wout]

Me too. Let's go for that then.

[dotnetCarpenter]

@Fuzzyma just a note, but reading the array flatten code:
It should be more efficient if we didn't create an Array instance, just to use its concat method.

array = array.reduce(function(prev, curr){
    return Array.prototype.concat.apply(prev, curr)
}, [])

I don't know how hot this code is. If it's only called a few times, then it makes no difference.

[Fuzzyma]

I dont think it makes a big difference but go forward and change it if you like. Actually we can test that with jsperf :D

[dotnetCarpenter]

Si! But you need a throw a lot of arrays after it to see a difference and last time I tried, jsperf broke.

[dotnetCarpenter]

Also I don't think the speed will change much, however I expect the memory usage to be quite different.

[Fuzzyma]

In this case I can argue that the array is thrown away in the next run so there is basically only one wasted array in memory every time :)

[dotnetCarpenter]

@Fuzzyma gotcha

[Fuzzyma]

Here you go:
https://jsperf.com/concat-vs-array-prototype-concat

Does not make much of a difference. FF is almost as twice as fast as chrome btw^^

[peterennis]

@dotnetCarpenter I would like to see #597 on the list for this release.

[Fuzzyma]

We are all no typescript user. We can put this on the list but only if you are willing to provide us with a pr for this

[peterennis]

OK. Sounds reasonable. I will look into the ts side as that is what I am using and open tickets as needed. Seems like the 3.0.0 timeline is more realistic.

[Fuzzyma]

@peterennis in 3.0 there will be api changes again so the ts file wont be up to date anyway. For user of the v2 branch we maybe should fix it, too. Many users wont switch to v3 immediately

[Fuzzyma]

@dotnetCarpenter with #626 all boxes now have a transform function which transforms the box with a given matrix.
I think we can check your last point when merged

[Fuzzyma]

issue with jsfiddle created: https://github.com/jsfiddle/jsfiddle-issues/issues/983

[Fuzzyma]

They already added it - thats great!