Merge pull request #561 from koenbok/issue/6457

Fix `copy()` crash on Frames containing frames. Fix https://github.co…
koenbok · Jan 23, 2018 · b6af600a515f8ec81fdf1e56050fe97a91b36be5 · b6af600
2 parents f6146fa + d365c74
commit b6af600a515f8ec81fdf1e56050fe97a91b36be5
Unified Split

Showing with 44 additions and 2 deletions.

+1 −1 framer/Layer.coffee

+6 −1 framer/SVGBaseLayer.coffee

+15 −0 framer/SVGLayer.coffee

+7 −0 framer/Utils.coffee

+9 −0 test/tests/SVGLayerTest.coffee

+6 −0 test/tests/UtilsTest.coffee
diff --git a/framer/Layer.coffee b/framer/Layer.coffee
@@ -986,7 +986,7 @@ class exports.Layer extends BaseClass

 		for child in @children
 			copiedChild = child.copy()
-			copiedChild.parent = layer
+			copiedChild.parent = layer if copiedChild isnt null

 		return layer

diff --git a/framer/SVGBaseLayer.coffee b/framer/SVGBaseLayer.coffee
@@ -1,10 +1,12 @@
 {LayerStyle} = require "./LayerStyle"
 {Layer, layerProperty} = require "./Layer"
 {Color} = require "./Color"
-
+Utils = require "./Utils"

 _svgMeasureElement = null

+denyCopy = ->
+	return Utils.throwInStudioOrWarnInProduction("SVGGroup and SVGPath do not support the `copy` method")

 getSVGMeasureElement = (constraints={}) ->
 	if not _svgMeasureElement?
@@ -190,3 +192,6 @@ class exports.SVGBaseLayer extends Layer
 	resetViewbox: =>
 		@_svg.setAttribute("viewBox", "0,0,#{@width},#{@height}")
 		@_svg.removeAttribute("viewBox")
+
+	copy: -> return denyCopy()
+	copySingle: -> return denyCopy()
diff --git a/framer/SVGLayer.coffee b/framer/SVGLayer.coffee
@@ -4,9 +4,12 @@
 {SVG} = require "./SVG"
 {SVGGroup} = require "./SVGGroup"
 {SVGPath} = require "./SVGPath"
+Utils = require "./Utils"

 class exports.SVGLayer extends Layer

+	@DenyCopyMessage: "SVGLayer doesn't support `copy` when the layer has one more children"
+
 	constructor: (options={}) ->
 		# Ugly: detect Vekter export with html intrinsic size
 		if options.htmlIntrinsicSize? and options.backgroundColor?
@@ -65,3 +68,15 @@ class exports.SVGLayer extends Layer
 				if value.parentNode?
 					value = value.cloneNode(true)
 				@_elementHTML.appendChild(value)
+
+	copy: ->
+		if @children.length > 0
+			return Utils.throwInStudioOrWarnInProduction(SVGLayer.DenyCopyMessage)
+		else
+			return super()
+
+	copySingle: ->
+		if @children.length > 0
+			return Utils.throwInStudioOrWarnInProduction(SVGLayer.DenyCopyMessage)
+		else
+			return super()
diff --git a/framer/Utils.coffee b/framer/Utils.coffee
@@ -1415,4 +1415,11 @@ Utils.textSize = (text, style={}, constraints={}) ->
 		height: rect.bottom - rect.top


+Utils.throwInStudioOrWarnInProduction = (message) ->
+	if Utils.isFramerStudio()
+		throw new Error(message)
+	# else
+	console.warn(message)
+	return null
+
 _.extend exports, Utils
diff --git a/test/tests/SVGLayerTest.coffee b/test/tests/SVGLayerTest.coffee
@@ -25,6 +25,15 @@ describe "SVGLayer", ->
 			b = a.copy()
 			a.htmlIntrinsicSize.should.eql b.htmlIntrinsicSize

+		it "should not copy SVGLayer that has children", ->
+			a = new SVGLayer
+				x: 123
+				y: 456
+				svg: '<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><path d="M 100 50 C 100 77.614 77.614 100 50 100 C 22.386 100 0 77.614 0 50 C 0 22.386 22.386 0 50 0" id="path" name="path" fill="transparent" stroke="#0AF"></path></svg>'
+
+			b = a.copy()
+			expect(b).to.be.null
+
 	describe "initializing", ->
 		it "should set clip to true by default", ->
 			a = new SVGLayer
diff --git a/test/tests/UtilsTest.coffee b/test/tests/UtilsTest.coffee
@@ -781,3 +781,9 @@ describe "Utils", ->
 				Utils.equal(new Color("red"), 0).should.be.false
 			it "should return false when only right side is a Color", ->
 				Utils.equal(3, new Color("red")).should.be.false
+
+	describe "throw or warn message", ->
+		it "should warn when in production", ->
+			Utils.throwInStudioOrWarnInProduction("test")
+			# not throwing means a succesful test. There's no way to mock being in Framer Studio afaik, that
+			# is, without resorting to adding a mocking lib like sinon.js, which is overkill for just this method.