From abcbaccbb46966446e47d8274b3b4d1d481b59a1 Mon Sep 17 00:00:00 2001 From: kfule Date: Fri, 15 Nov 2024 20:51:29 +0900 Subject: [PATCH 1/3] updateStyle(): use setProperty() when css vars and dashed-properties, fixes #2989 --- render/render.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/render/render.js b/render/render.js index 6888d06f5..6ecb38ee5 100644 --- a/render/render.js +++ b/render/render.js @@ -794,7 +794,7 @@ module.exports = function() { for (var key in style) { var value = style[key] if (value != null) { - if (key[0] === "-" && key[1] === "-") element.style.setProperty(key, String(value)) + if (key.includes("-")) element.style.setProperty(key, String(value)) else element.style[key] = String(value) } } @@ -804,14 +804,14 @@ module.exports = function() { for (var key in style) { var value = style[key] if (value != null && (value = String(value)) !== String(old[key])) { - if (key[0] === "-" && key[1] === "-") element.style.setProperty(key, value) + if (key.includes("-")) element.style.setProperty(key, value) else element.style[key] = value } } // Remove style properties that no longer exist for (var key in old) { if (old[key] != null && style[key] == null) { - if (key[0] === "-" && key[1] === "-") element.style.removeProperty(key) + if (key.includes("-")) element.style.removeProperty(key) else element.style[key] = "" } } From afb210734e4a73b292b2ee495cd72f8ba7578764 Mon Sep 17 00:00:00 2001 From: kfule Date: Sat, 16 Nov 2024 19:19:19 +0900 Subject: [PATCH 2/3] add a test spying on a style setter --- render/tests/test-updateElement.js | 73 ++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/render/tests/test-updateElement.js b/render/tests/test-updateElement.js index eb41e06f4..bc99457a1 100644 --- a/render/tests/test-updateElement.js +++ b/render/tests/test-updateElement.js @@ -257,6 +257,79 @@ o.spec("updateElement", function() { } }) + o("use style property setter only when cameCase keys", function() { + var spySetProperty = o.spy() + var spyRemoveProperty = o.spy() + var spyDashed1 = o.spy() + var spyDashed2 = o.spy() + var spyDashed3 = o.spy() + var spyCamelCase1 = o.spy() + var spyCamelCase2 = o.spy() + + var f = $window.document.createElement + $window.document.createElement = function(tag, is){ + var el = f(tag, is) + + var style = {} + Object.defineProperties(style, { + setProperty: {value: spySetProperty}, + removeProperty: {value: spyRemoveProperty}, + /* eslint-disable accessor-pairs */ + "background-color": {set: spyDashed1}, + "-webkit-border-radius": {set: spyDashed2}, + "--foo": {set: spyDashed3}, + backgroundColor: {set: spyCamelCase1}, + color: {set: spyCamelCase2} + /* eslint-enable accessor-pairs */ + }) + + Object.defineProperty(el, "style", {value: style}) + return el + } + + // sets dashed properties + render(root, m("a", { + style: { + "background-color": "red", + "-webkit-border-radius": "10px", + "--foo": "bar" + } + })) + + // sets camelCase properties and removes dashed properties + render(root, m("a", { + style: { + backgroundColor: "green", + color: "red", + } + })) + + // updates "color" and removes "backgroundColor" + render(root, m("a", {style: {color: "blue"}})) + + // setProperty (for dashed properties) + o(spySetProperty.callCount).equals(3) + o(spySetProperty.calls[0].args).deepEquals(["background-color", "red"]) + o(spySetProperty.calls[1].args).deepEquals(["-webkit-border-radius", "10px"]) + o(spySetProperty.calls[2].args).deepEquals(["--foo", "bar"]) + + // removeProperty (for dashed properties) + o(spyRemoveProperty.callCount).equals(3) + o(spyRemoveProperty.calls[0].args).deepEquals(["background-color"]) + o(spyRemoveProperty.calls[1].args).deepEquals(["-webkit-border-radius"]) + o(spyRemoveProperty.calls[2].args).deepEquals(["--foo"]) + + // property setter (for camelCase properties) + o(spyDashed1.callCount).equals(0) + o(spyDashed2.callCount).equals(0) + o(spyDashed3.callCount).equals(0) + o(spyCamelCase1.callCount).equals(2) // set and remove + o(spyCamelCase2.callCount).equals(2) // set and update + o(spyCamelCase1.calls[0].args).deepEquals(["green"]) + o(spyCamelCase1.calls[1].args).deepEquals([""]) + o(spyCamelCase2.calls[0].args).deepEquals(["red"]) + o(spyCamelCase2.calls[1].args).deepEquals(["blue"]) + }) o("replaces el", function() { var vnode = m("a") var updated = m("b") From b4af2f61533d331966ca3d2d994268b4f288b1fe Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 18 Nov 2024 19:40:13 +0900 Subject: [PATCH 3/3] change domMock setProperty/removeProperty to writable and simplify wrapping properties --- render/tests/test-updateElement.js | 68 ++++++++++++++---------------- test-utils/domMock.js | 18 +++++--- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/render/tests/test-updateElement.js b/render/tests/test-updateElement.js index bc99457a1..771b4a405 100644 --- a/render/tests/test-updateElement.js +++ b/render/tests/test-updateElement.js @@ -266,26 +266,20 @@ o.spec("updateElement", function() { var spyCamelCase1 = o.spy() var spyCamelCase2 = o.spy() - var f = $window.document.createElement - $window.document.createElement = function(tag, is){ - var el = f(tag, is) - - var style = {} - Object.defineProperties(style, { - setProperty: {value: spySetProperty}, - removeProperty: {value: spyRemoveProperty}, - /* eslint-disable accessor-pairs */ - "background-color": {set: spyDashed1}, - "-webkit-border-radius": {set: spyDashed2}, - "--foo": {set: spyDashed3}, - backgroundColor: {set: spyCamelCase1}, - color: {set: spyCamelCase2} - /* eslint-enable accessor-pairs */ - }) - - Object.defineProperty(el, "style", {value: style}) - return el - } + render(root, m("a")) + var el = root.firstChild + + el.style.setProperty = spySetProperty + el.style.removeProperty = spyRemoveProperty + Object.defineProperties(el.style, { + /* eslint-disable accessor-pairs */ + "background-color": {set: spyDashed1}, + "-webkit-border-radius": {set: spyDashed2}, + "--foo": {set: spyDashed3}, + backgroundColor: {set: spyCamelCase1}, + color: {set: spyCamelCase2} + /* eslint-enable accessor-pairs */ + }) // sets dashed properties render(root, m("a", { @@ -295,6 +289,10 @@ o.spec("updateElement", function() { "--foo": "bar" } })) + o(spySetProperty.callCount).equals(3) + o(spySetProperty.calls[0].args).deepEquals(["background-color", "red"]) + o(spySetProperty.calls[1].args).deepEquals(["-webkit-border-radius", "10px"]) + o(spySetProperty.calls[2].args).deepEquals(["--foo", "bar"]) // sets camelCase properties and removes dashed properties render(root, m("a", { @@ -303,32 +301,30 @@ o.spec("updateElement", function() { color: "red", } })) + o(spyCamelCase1.callCount).equals(1) + o(spyCamelCase2.callCount).equals(1) + o(spyCamelCase1.calls[0].args).deepEquals(["green"]) + o(spyCamelCase2.calls[0].args).deepEquals(["red"]) + o(spyRemoveProperty.callCount).equals(3) + o(spyRemoveProperty.calls[0].args).deepEquals(["background-color"]) + o(spyRemoveProperty.calls[1].args).deepEquals(["-webkit-border-radius"]) + o(spyRemoveProperty.calls[2].args).deepEquals(["--foo"]) // updates "color" and removes "backgroundColor" render(root, m("a", {style: {color: "blue"}})) + o(spyCamelCase1.callCount).equals(2) // set and remove + o(spyCamelCase2.callCount).equals(2) // set and update + o(spyCamelCase1.calls[1].args).deepEquals([""]) + o(spyCamelCase2.calls[1].args).deepEquals(["blue"]) - // setProperty (for dashed properties) + // unchanged by camelCase properties o(spySetProperty.callCount).equals(3) - o(spySetProperty.calls[0].args).deepEquals(["background-color", "red"]) - o(spySetProperty.calls[1].args).deepEquals(["-webkit-border-radius", "10px"]) - o(spySetProperty.calls[2].args).deepEquals(["--foo", "bar"]) - - // removeProperty (for dashed properties) o(spyRemoveProperty.callCount).equals(3) - o(spyRemoveProperty.calls[0].args).deepEquals(["background-color"]) - o(spyRemoveProperty.calls[1].args).deepEquals(["-webkit-border-radius"]) - o(spyRemoveProperty.calls[2].args).deepEquals(["--foo"]) - // property setter (for camelCase properties) + // never calls dashed property setter o(spyDashed1.callCount).equals(0) o(spyDashed2.callCount).equals(0) o(spyDashed3.callCount).equals(0) - o(spyCamelCase1.callCount).equals(2) // set and remove - o(spyCamelCase2.callCount).equals(2) // set and update - o(spyCamelCase1.calls[0].args).deepEquals(["green"]) - o(spyCamelCase1.calls[1].args).deepEquals([""]) - o(spyCamelCase2.calls[0].args).deepEquals(["red"]) - o(spyCamelCase2.calls[1].args).deepEquals(["blue"]) }) o("replaces el", function() { var vnode = m("a") diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 5536b49fd..3a86504af 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -284,12 +284,18 @@ module.exports = function(options) { getPropertyValue: {value: function(key){ return style[key] }}, - removeProperty: {value: function(key){ - style[key] = style[camelCase(key)] = "" - }}, - setProperty: {value: function(key, value){ - style[key] = style[camelCase(key)] = value - }} + removeProperty: { + writable: true, + value: function(key){ + style[key] = style[camelCase(key)] = "" + } + }, + setProperty: { + writable: true, + value: function(key, value){ + style[key] = style[camelCase(key)] = value + } + } }) var events = {} var element = {