Skip to content

Commit

Permalink
fix(toPoints): do not add extra point for z if first and previous poi…
Browse files Browse the repository at this point in the history 
…nts are the same

BREAKING CHANGE: no longer adds an additional point for path z command if previous and first points
already the same
  • Loading branch information
@colinmeinke
colinmeinke committed May 13, 2017
1 parent 464f0a6 commit 90cb1b2
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/toPoints.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,13 @@ const getPointsFromPath = ({ d }) => {
const upperCaseCommand = command.toUpperCase()
const commandLength = commandLengths[ upperCaseCommand ]
const relative = isRelative(command)
const prevPoint = i === 0 ? null : points[ points.length - 1 ]

if (commandLength > 0) {
const commandParams = params.shift()
const iterations = commandParams.length / commandLength

for (let j = 0; j < iterations; j++) {
const prevPoint = i === 0 ? null : points[ points.length - 1 ]

switch (upperCaseCommand) {
case 'M':
const x = (relative && prevPoint ? prevPoint.x : 0) + commandParams.shift()
Expand Down Expand Up @@ -246,7 +245,9 @@ const getPointsFromPath = ({ d }) => {
}
}
} else {
points.push({ x: moveTo.x, y: moveTo.y })
if (prevPoint.x !== moveTo.x || prevPoint.y !== moveTo.y) {
points.push({ x: moveTo.x, y: moveTo.y })
}
}
}

Expand Down
17 changes: 17 additions & 0 deletions test/toPoints.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,20 @@ test('should return correct points of a g', () => {

expect(points).toEqual(expectedPoints)
})

test('should not add an additional point for z if last point equals first', () => {
const shape = {
type: 'path',
d: 'M30,45A5,5,0,0,1,30,55A5,5,0,0,1,30,45Z'
}

const expectedPoints = [
{ x: 30, y: 45, moveTo: true },
{ x: 30, y: 55, curve: { type: 'arc', rx: 5, ry: 5, sweepFlag: 1 } },
{ x: 30, y: 45, curve: { type: 'arc', rx: 5, ry: 5, sweepFlag: 1 } }
]

const points = toPoints(shape)

expect(points).toEqual(expectedPoints)
})

5 comments on commit 90cb1b2

@dalisoft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colinmeinke Thanks, Wow. This is where has been my bug month ago, i just leaved it for later, but you fixed it. Nice. About svg-points and points - you want implement wilderness-core latest script to these apps, for users make own library which core is svg-points and points? Thanks

@colinmeinke
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dalisoft cool - I'm not sure what you mean by:

you want implement wilderness-core latest script to these apps, for users make own library which core is svg-points and points?

Can you explain in a different way?

@dalisoft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In wilderness-core i've seen code piece where normalises non-group <-> group, curve normalisation and much more for complex transform including transform[] syntax. Would be nice, if you implement these in points while calling add function. For some users (like me) to maintain own morph/svg library while core would be svg-points/points. Thanks.

There i describe why i asked this:
I solved my complex morph tween (for missing points) with nearest and worked good, but for some larger/more subpath shapes this makes like game, but your solution makes it more real and live. So i decided to maintain for create my own library (private will be, or public, not sure, credit tags always i give).

I want like this syntax for me.

import { toPath, toPoints } from 'svg-points';
import Points from 'points';
import { normaliseAndDoRealShape } from 'wilderness-core';
import TWEEN from 'es6-tween';

class Morph {
constructor () {
this._path = {start:null,end:null};
}
startPath (tagOrNode) {
this._path.node = tagOrNode;
this._path.start = tagOrNode.getAttribute("d");
}
endPath (tagOrNode) {
this._path.end= tagOrNode.getAttribute("d");
}
start () {
const { start, end } = normaliseAndDoRealShape(this._path.start, this._path.end);
return new TWEEN.Tween({d:start}).to({d:end}, 2000).on('update', (obj) => {
this._path.node.setAttribute("d", obj.d);
}).start();
}
}

I hope you understand why i mean. This way gives me much flexibility as i worked with tween for 5-years even i maintaining ES6 version of this lib (it's not ad).

Anyway i learned ES6 from you, as your code motivated me. Thanks a lot. Thanks for reply too

@colinmeinke
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I understand what you mean now, thanks for explaining. Hmm, I don't think I want to make those functions part of the public api. However, you could use the shape function, which will equalise shapes and returns an object that you could then get what you need from. Something like this:

import { shape } from 'wilderness-core'

const { keyframes } = shape(shape1, shape2)

const frameShape1 = keyframes[ 0 ].frameShape
const frameShape2 = keyframes[ 1 ].frameShape

A FrameShape is an object that include the following props:

  • points - the shape's points (unless it's a group shape)
  • childFrameShapes - and array of children (only if the shape is a group)

I hope that helps.

@dalisoft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for giving code to get what i need. Thanks for amazing library.

Please sign in to comment.