Skip to content

Commit

Permalink
Restore logic to not remove defaults from elements with id attributes.
Browse files Browse the repository at this point in the history
  • Loading branch information
johnkenny54 committed Sep 19, 2024
1 parent 9102aab commit 5ef2264
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 4 deletions.
3 changes: 2 additions & 1 deletion docs/plugins/removeUnknownsAndDefaults.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ Elements are removed if:
Attributes set to a default value are removed if:

- They are not overriding a value set by an ancestor, and
- they are not in an element which is referenced by a `<use>` element (or a child of any such element)
- they are not in an element that has an id (e.g., a `linearGradient` or similar element which may be referenced elsewhere)
- they are not in an element which is referenced by a `<use>` element (or a child of any such element).
8 changes: 6 additions & 2 deletions plugins/removeUnknownsAndDefaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export const fn = (root, params, info) => {
const allowedChildren = allowedChildrenPerElement.get(
parentNode.name,
);
if (allowedChildren == null || allowedChildren.size === 0) {
if (!allowedChildren || allowedChildren.size === 0) {
// TODO: DO WE NEED THIS CHECK? SHOULDN'T IT HAVE BEEN HANDLED BY THE PARENT IN THE ELSE BLOCK BELOW?
// remove unknown elements
if (allowedChildrenPerElement.get(node.name) == null) {
Expand Down Expand Up @@ -251,8 +251,13 @@ export const fn = (root, params, info) => {
) {
delete node.attributes[name];
}

// Don't remove default attributes from elements with an id attribute; they may be linearGradient, etc.
// where the attribute serves a purpose. If the id is unnecessary, it will be removed by another plugin
// and the attribute will then be removable.
if (
defaultAttrs &&
!node.attributes.id &&
attributesDefaults &&
attributesDefaults.get(name) === value
) {
Expand All @@ -264,7 +269,6 @@ export const fn = (root, params, info) => {
saveForUsageCheck(node, name);
}
}
// TODO: DO WE NEED THE ID CHECK? SEEMS LIKE THIS IS HANDLED BY CHECKING FOR USE
if (uselessOverrides && node.attributes.id == null) {
const computedValue = computedParentStyle
? computedParentStyle.get(name)
Expand Down
2 changes: 1 addition & 1 deletion test/plugins/removeUnknownsAndDefaults.01.svg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@

<svg xmlns="http://www.w3.org/2000/svg" xmlns:test="http://" y="10" test:attr="val" xml:space="preserve">
<rect/>
<rect id="black-rect"/>
<rect fill="#000" id="black-rect"/>
</svg>
28 changes: 28 additions & 0 deletions test/plugins/removeUnknownsAndDefaults.19.svg.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
Preserve defaults on linearGradient elements.

===

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 128 128">
<linearGradient y2="69" x2="220" y1="69" x1="26" gradientUnits="userSpaceOnUse" id="b">
<stop id="stop5458" style="stop-color:#959ba3;stop-opacity:1" offset="0" />
<stop offset="0.09090909" style="stop-color:#f4f5f5;stop-opacity:1;" id="stop5460" />
<stop id="stop5462" style="stop-color:#9da5ac;stop-opacity:1;" offset="0.18181819" />
<stop id="stop5464" style="stop-color:#8b96a1;stop-opacity:1" offset="1" />
</linearGradient>
<linearGradient href="#b" id="a" gradientUnits="userSpaceOnUse" spreadMethod="reflect" x1="0" y1="90.18235" x2="63.9991" y2="90.18235" gradientTransform="matrix(1,0,0,0.91756261,0,21.247322)" />
<path style="fill:url(#a);stroke-width:0;" d="m 0,96 0,5.33334 0,13.33332 C 0,117.62418 2.5892726,120 5.8125,120 l 116.375,0 C 125.41073,120 128,117.62418 128,114.66666 L 128,101.33334 128,96 122.1875,96 5.8125,96 z"/>
</svg>

@@@

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 128 128">
<linearGradient y2="69" x2="220" y1="69" x1="26" gradientUnits="userSpaceOnUse" id="b">
<stop id="stop5458" style="stop-color:#959ba3;stop-opacity:1" offset="0"/>
<stop offset="0.09090909" style="stop-color:#f4f5f5;stop-opacity:1;" id="stop5460"/>
<stop id="stop5462" style="stop-color:#9da5ac;stop-opacity:1;" offset="0.18181819"/>
<stop id="stop5464" style="stop-color:#8b96a1;stop-opacity:1" offset="1"/>
</linearGradient>
<linearGradient href="#b" id="a" gradientUnits="userSpaceOnUse" spreadMethod="reflect" x1="0" y1="90.18235" x2="63.9991" y2="90.18235" gradientTransform="matrix(1,0,0,0.91756261,0,21.247322)"/>
<path style="fill:url(#a);stroke-width:0;" d="m 0,96 0,5.33334 0,13.33332 C 0,117.62418 2.5892726,120 5.8125,120 l 116.375,0 C 125.41073,120 128,117.62418 128,114.66666 L 128,101.33334 128,96 122.1875,96 5.8125,96 z"/>
</svg>

0 comments on commit 5ef2264

Please sign in to comment.