Skip to content

Commit

Permalink
ids are not generated upon creation anymore
Browse files Browse the repository at this point in the history
Instead they are generated when requested (#559)
  • Loading branch information
Fuzzyma committed Apr 23, 2017
1 parent ab1c07e commit 54362a8
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 65 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ The document follows the conventions described in [“Keep a CHANGELOG”](http:
- `svg()` will now return the element without svg-wrapper
- new constructor signature for `SVG.Image` and `load()`: `container.image(src, callback) / image.load(src, callback)`
- changed `style()` to `css()`. Now accepts array as input and returns object when no argument given (#517)
- ids are not generated upon creation anymore. Instead they are generated when requested (#559)

### Fixed
- fixed a bug in clipping and masking where empty nodes persists after removal -> __TODO!__
Expand Down
30 changes: 17 additions & 13 deletions dist/svg.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* @copyright Wout Fierens <[email protected]>
* @license MIT
*
* BUILT: Sat Apr 22 2017 22:14:55 GMT+0200 (Mitteleuropäische Sommerzeit)
* BUILT: Sun Apr 23 2017 12:52:05 GMT+0200 (Mitteleuropäische Sommerzeit)
*/;
(function(root, factory) {
/* istanbul ignore next */
Expand Down Expand Up @@ -59,12 +59,7 @@ SVG.eid = function(name) {
// Method for element creation
SVG.create = function(name) {
// create element
var element = document.createElementNS(this.ns, name)

// apply unique id
element.setAttribute('id', this.eid(name))

return element
return document.createElementNS(this.ns, name)
}

// Method for extending objects
Expand Down Expand Up @@ -1075,7 +1070,7 @@ SVG.Element = SVG.invent({
.height(new SVG.Number(p.height))
}
// Clone element
, clone: function(parent, withData) {
, clone: function(parent) {
// write dom data to the dom so the clone can pickup the data
this.writeDataToDom()

Expand Down Expand Up @@ -1111,6 +1106,12 @@ SVG.Element = SVG.invent({
}
// Get / set id
, id: function(id) {
// generate new id if no id set
if(typeof id == 'undefined' && !this.node.id) {
this.node.id = SVG.eid(this.type)
}

// dont't set directly width this.node.id to make `null` work correctly
return this.attr('id', id)
}
// Checks whether the given point inside the bounding box of the element
Expand All @@ -1136,7 +1137,7 @@ SVG.Element = SVG.invent({
}
// Return id on string conversion
, toString: function() {
return this.attr('id')
return this.id()
}
// Return array of classes on the node
, classes: function() {
Expand Down Expand Up @@ -3352,7 +3353,7 @@ SVG.Mask = SVG.invent({
}

, targets: function() {
return SVG.select('svg [mask*="' +this.id() +'"]')
return SVG.select('svg [mask*="' + this.id() + '"]')
}
}

Expand All @@ -3373,7 +3374,7 @@ SVG.extend(SVG.Element, {
var masker = element instanceof SVG.Mask ? element : this.parent().mask().add(element)

// apply mask
return this.attr('mask', 'url("#' + masker.attr('id') + '")')
return this.attr('mask', 'url("#' + masker.id() + '")')
}
// Unmask element
, unmask: function() {
Expand Down Expand Up @@ -3426,7 +3427,7 @@ SVG.extend(SVG.Element, {
var clipper = element instanceof SVG.ClipPath ? element : this.parent().clip().add(element)

// apply mask
return this.attr('clip-path', 'url("#' + clipper.attr('id') + '")')
return this.attr('clip-path', 'url("#' + clipper.id() + '")')
}
// Unclip element
, unclip: function() {
Expand Down Expand Up @@ -4939,7 +4940,10 @@ function assignNewId(node) {
if (node.childNodes[i] instanceof window.SVGElement)
assignNewId(node.childNodes[i])

return SVG.adopt(node).id(SVG.eid(node.nodeName))
if(node.id)
return SVG.adopt(node).id(SVG.eid(node.nodeName))

return SVG.adopt(node)
}

// Add more bounding box properties
Expand Down
4 changes: 2 additions & 2 deletions dist/svg.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion spec/spec/clip.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('ClipPath', function() {
})

it('sets the "clip-path" attribute on the cliped element with the clip id', function() {
expect(rect.attr('clip-path')).toBe('url("#' + circle.parent().attr('id') + '")')
expect(rect.attr('clip-path')).toBe('url("#' + circle.parent().id() + '")')
})

it('references the clip element in the masked element', function() {
Expand Down
35 changes: 26 additions & 9 deletions spec/spec/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,29 @@ describe('Element', function() {
rect = draw.rect(100,100)
})

it('generates an id when getting if no id is set on the element', function() {
expect(rect.attr('id')).toBe(undefined)
expect(rect.id()).not.toBe(null)
expect(rect.node.id).not.toBe(null)
})
it('increases the global id sequence', function() {
var did = SVG.did
rect.id()

expect(did + 1).toBe(SVG.did)
})
it('adds a unique id containing the node name', function() {
var did = SVG.did
rect.id()

expect(rect.attr('id')).toBe('SvgjsRect' + did)
})
it('gets the value if the id attribute without an argument', function() {
expect(rect.id()).toBe(rect.attr('id'))
})
it('sets the value of the id', function() {
rect.id('new_id')
expect(rect.attr('id')).toBe('new_id')
expect(rect.id()).toBe('new_id')
})
})

Expand Down Expand Up @@ -644,17 +661,17 @@ describe('Element', function() {
})
it('assigns a new id to the cloned element', function() {
clone = rect.clone()
expect(clone.attr('id')).not.toBe(rect.attr('id'))
expect(clone.id()).not.toBe(rect.id())
})
it('copies all child nodes as well', function() {
clone = group.clone()
expect(clone.children().length).toBe(group.children().length)
})
it('assigns a new id to cloned child elements', function() {
clone = group.clone()
expect(clone.attr('id')).not.toEqual(group.attr('id'))
expect(clone.get(0).attr('id')).not.toBe(group.get(0).attr('id'))
expect(clone.get(1).attr('id')).not.toBe(group.get(1).attr('id'))
expect(clone.id()).not.toEqual(group.id())
expect(clone.get(0).id()).not.toBe(group.get(0).id())
expect(clone.get(1).id()).not.toBe(group.get(1).id())
})
it('inserts the clone after the cloned element', function() {
clone = rect.clone()
Expand All @@ -677,7 +694,7 @@ describe('Element', function() {
describe('toString()', function() {
it('returns the element id', function() {
var rect = draw.rect(100,100).center(321,567).fill('#f06')
expect(rect + '').toBe(rect.attr('id'))
expect(rect + '').toBe(rect.id())
})
})

Expand Down Expand Up @@ -809,13 +826,13 @@ describe('Element', function() {
// Test for different browsers namely Firefox and Chrome
expect(
// IE
toBeTested === '<svg xmlns:svgjs="http://svgjs.com/svgjs" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" xmlns="http://www.w3.org/2000/svg" height="100" width="100" id="' + draw.id() + '"><rect height="100" width="100"></rect><circle fill="#ff0066" cy="50" cx="50" r="50"></circle></svg>'
toBeTested === '<svg xmlns:svgjs="http://svgjs.com/svgjs" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" xmlns="http://www.w3.org/2000/svg" height="100" width="100"><rect height="100" width="100"></rect><circle fill="#ff0066" cy="50" cx="50" r="50"></circle></svg>'

// Firefox
|| toBeTested === '<svg id="' + draw.id() + '" width="100" height="100" xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:svgjs="http://svgjs.com/svgjs"><rect width="100" height="100"></rect><circle r="50" cx="50" cy="50" fill="#ff0066"></circle></svg>'
|| toBeTested === '<svg width="100" height="100" xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:svgjs="http://svgjs.com/svgjs"><rect width="100" height="100"></rect><circle r="50" cx="50" cy="50" fill="#ff0066"></circle></svg>'

// svgdom
|| toBeTested === '<svg id="' + draw.id() + '" xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:svgjs="http://svgjs.com/svgjs" width="100" height="100"><svg id="SvgjsSvg1002" width="2" height="0" style="overflow: hidden; top: -100%; left: -100%; position: absolute; opacity: 0"><polyline id="SvgjsPolyline1003" points="10,10 20,10 30,10"></polyline><path id="SvgjsPath1004" d="M80 80A45 45 0 0 0 125 125L125 80Z "></path></svg><rect width="100" height="100"></rect><circle r="50" cx="50" cy="50" fill="#ff0066"></circle></svg>'
|| toBeTested === '<svg xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:svgjs="http://svgjs.com/svgjs" width="100" height="100"><svg id="SvgjsSvg1002" width="2" height="0" style="overflow: hidden; top: -100%; left: -100%; position: absolute; opacity: 0"><polyline id="SvgjsPolyline1003" points="10,10 20,10 30,10"></polyline><path id="SvgjsPath1004" d="M80 80A45 45 0 0 0 125 125L125 80Z "></path></svg><rect width="100" height="100"></rect><circle r="50" cx="50" cy="50" fill="#ff0066"></circle></svg>'
).toBeTruthy()

})
Expand Down
6 changes: 3 additions & 3 deletions spec/spec/gradient.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Gradient', function() {

describe('fill()', function() {
it('returns the id of the gradient wrapped in url()', function() {
expect(gradient.fill()).toBe('url(#' + gradient.attr('id') + ')')
expect(gradient.fill()).toBe('url(#' + gradient.id() + ')')
})
})

Expand Down Expand Up @@ -67,11 +67,11 @@ describe('Gradient', function() {

describe('toString()', function() {
it('returns the id of the gradient wrapped in url()', function() {
expect(gradient + '').toBe('url(#' + gradient.attr('id') + ')')
expect(gradient + '').toBe('url(#' + gradient.id() + ')')
})
it('is called when instance is passed as an attribute value', function() {
rect.attr('fill', gradient)
expect(rect.attr('fill')).toBe('url(#' + gradient.attr('id') + ')')
expect(rect.attr('fill')).toBe('url(#' + gradient.id() + ')')
})
})

Expand Down
2 changes: 1 addition & 1 deletion spec/spec/mask.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('Mask', function() {
})

it('sets the "mask" attribute on the masked element with the mask id', function() {
expect(rect.attr('mask')).toBe('url("#' + circle.parent().attr('id') + '")')
expect(rect.attr('mask')).toBe('url("#' + circle.parent().id() + '")')
})

it('references the mask element in the masked element', function() {
Expand Down
8 changes: 4 additions & 4 deletions spec/spec/pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('Pattern', function() {

describe('fill()', function() {
it('returns the id of the pattern wrapped in url()', function() {
expect(pattern.fill()).toBe('url(#' + pattern.attr('id') + ')')
expect(pattern.fill()).toBe('url(#' + pattern.id() + ')')
})
})

Expand All @@ -37,15 +37,15 @@ describe('Pattern', function() {

describe('toString()', function() {
it('returns the id of the pattern wrapped in url()', function() {
expect(pattern + '').toBe('url(#' + pattern.attr('id') + ')')
expect(pattern + '').toBe('url(#' + pattern.id() + ')')
})
it('is called when instance is passed as an attribute value', function() {
rect.attr('fill', pattern)
expect(rect.attr('fill')).toBe('url(#' + pattern.attr('id') + ')')
expect(rect.attr('fill')).toBe('url(#' + pattern.id() + ')')
})
it('is called when instance is passed in a fill() method', function() {
rect.fill(pattern)
expect(rect.attr('fill')).toBe('url(#' + pattern.attr('id') + ')')
expect(rect.attr('fill')).toBe('url(#' + pattern.id() + ')')
})
})

Expand Down
4 changes: 2 additions & 2 deletions spec/spec/selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ describe('Selector', function() {
it('gets an element\'s instance by id', function() {
var rect = draw.rect(111, 333)

expect(SVG.get(rect.attr('id'))).toBe(rect)
expect(SVG.get(rect.id())).toBe(rect)
})
it('makes all the element\'s methods available', function() {
var element = draw.group()
, got = SVG.get(element.attr('id'))
, got = SVG.get(element.id())

expect(got.attr()).toEqual(element.attr())
})
Expand Down
12 changes: 0 additions & 12 deletions spec/spec/svg.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,6 @@ describe('SVG', function() {

expect(element.nodeName).toBe('rect')
})
it('increases the global id sequence', function() {
var did = SVG.did
, element = SVG.create('rect')

expect(did + 1).toBe(SVG.did)
})
it('adds a unique id containing the node name', function() {
var did = SVG.did
, element = SVG.create('rect')

expect(element.getAttribute('id')).toBe('SvgjsRect' + did)
})
})

describe('extend()', function() {
Expand Down
2 changes: 1 addition & 1 deletion src/clip.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ SVG.extend(SVG.Element, {
var clipper = element instanceof SVG.ClipPath ? element : this.parent().clip().add(element)

// apply mask
return this.attr('clip-path', 'url("#' + clipper.attr('id') + '")')
return this.attr('clip-path', 'url("#' + clipper.id() + '")')
}
// Unclip element
, unclip: function() {
Expand Down
10 changes: 8 additions & 2 deletions src/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ SVG.Element = SVG.invent({
.height(new SVG.Number(p.height))
}
// Clone element
, clone: function(parent, withData) {
, clone: function(parent) {
// write dom data to the dom so the clone can pickup the data
this.writeDataToDom()

Expand Down Expand Up @@ -94,6 +94,12 @@ SVG.Element = SVG.invent({
}
// Get / set id
, id: function(id) {
// generate new id if no id set
if(typeof id == 'undefined' && !this.node.id) {
this.node.id = SVG.eid(this.type)
}

// dont't set directly width this.node.id to make `null` work correctly
return this.attr('id', id)
}
// Checks whether the given point inside the bounding box of the element
Expand All @@ -119,7 +125,7 @@ SVG.Element = SVG.invent({
}
// Return id on string conversion
, toString: function() {
return this.attr('id')
return this.id()
}
// Return array of classes on the node
, classes: function() {
Expand Down
17 changes: 10 additions & 7 deletions src/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function matches(el, selector) {
}

// Convert dash-separated-string to camelCase
function camelCase(s) {
function camelCase(s) {
return s.toLowerCase().replace(/-(.)/g, function(m, g) {
return g.toUpperCase()
})
Expand All @@ -49,7 +49,7 @@ function capitalize(s) {
return s.charAt(0).toUpperCase() + s.slice(1)
}

// Ensure to six-based hex
// Ensure to six-based hex
function fullHex(hex) {
return hex.length == 4 ?
[ '#',
Expand All @@ -69,13 +69,13 @@ function compToHex(comp) {
function proportionalSize(element, width, height) {
if (width == null || height == null) {
var box = element.bbox()

if (width == null)
width = box.width / box.height * height
else if (height == null)
height = box.height / box.width * width
}

return {
width: width
, height: height
Expand All @@ -99,7 +99,7 @@ function arrayToMatrix(a) {
function parseMatrix(matrix) {
if (!(matrix instanceof SVG.Matrix))
matrix = new SVG.Matrix(matrix)

return matrix
}

Expand Down Expand Up @@ -142,7 +142,7 @@ function arrayToString(a) {
}
}
}

return s + ' '
}

Expand All @@ -153,7 +153,10 @@ function assignNewId(node) {
if (node.childNodes[i] instanceof window.SVGElement)
assignNewId(node.childNodes[i])

return SVG.adopt(node).id(SVG.eid(node.nodeName))
if(node.id)
return SVG.adopt(node).id(SVG.eid(node.nodeName))

return SVG.adopt(node)
}

// Add more bounding box properties
Expand Down
Loading

0 comments on commit 54362a8

Please sign in to comment.