Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

moveElemsStylesToGroups: update attributes as well #44

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/svgo/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export const cleanupOutData = (data, params, command) => {
/**
* @param {number} n
* @param {number} m
* @deprecated
*/
export function exactAdd(n, m) {
const d1 = getNumberOfDecimalDigits(n);
Expand All @@ -136,6 +137,7 @@ export function exactAdd(n, m) {
/**
* @param {number} n
* @param {number} m
* @deprecated
*/
export function exactMul(n, m) {
const d1 = getNumberOfDecimalDigits(n);
Expand Down
56 changes: 56 additions & 0 deletions plugins/_styles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { getStyleDeclarations } from '../lib/css-tools.js';
import { svgAttTransformToCSS, svgToString } from '../lib/svg-parse-att.js';
import { inheritableAttrs } from './_collections.js';

export const TRANSFORM_PROP_NAMES = ['transform', 'transform-origin'];

/**
* @param {import('../lib/types.js').XastElement} element
* @returns {import('../lib/types.js').CSSDeclarationMap}
*/
export function getInheritableProperties(element) {
/** @type {import('../lib/types.js').CSSDeclarationMap} */
const props = new Map();

// Gather all inheritable attributes.
for (const [k, v] of Object.entries(element.attributes)) {
const value = getSVGAttributeValue(v);
if (inheritableAttrs.has(k)) {
props.set(k, { value: svgToString(value), important: false });
} else if (k === 'transform') {
const cssValue = svgAttTransformToCSS(value);
if (cssValue) {
props.set(k, cssValue);
}
} else if (TRANSFORM_PROP_NAMES.includes(k)) {
props.set(k, { value: svgToString(value), important: false });
}
}

// Overwrite with inheritable properties.
const styleProps = getStyleDeclarations(element);
if (styleProps) {
styleProps.forEach((v, k) => {
if (inheritableAttrs.has(k) || TRANSFORM_PROP_NAMES.includes(k)) {
if (v === null) {
props.delete(k);
} else {
props.set(k, v);
}
}
});
}

return props;
}

/**
* @param {string|import('../lib/types.js').SVGAttValue} v
* @returns {import('../lib/types.js').SVGAttValue}
*/
function getSVGAttributeValue(v) {
if (typeof v === 'string') {
return { strVal: v };
}
return v;
}
59 changes: 2 additions & 57 deletions plugins/createGroups.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import {
svgAttTransformToCSS,
svgSetAttributeValue,
} from '../lib/svg-parse-att.js';
import { svgSetAttributeValue } from '../lib/svg-parse-att.js';
import { cssToString, cssTransformToSVGAtt } from '../lib/css-parse-decl.js';
import { getStyleDeclarations } from '../lib/css-tools.js';
import { writeStyleAttribute } from '../lib/css.js';
import { svgToString } from '../lib/svg-parse-att.js';
import { getHrefId } from '../lib/svgo/tools.js';
import { inheritableAttrs } from './_collections.js';
import { getInheritableProperties } from './_styles.js';

export const name = 'createGroups';
export const description =
Expand Down Expand Up @@ -239,54 +235,3 @@ function createGroups(element, usedIds, elementsToCheck) {
}
}
}

/**
* @param {import('../lib/types.js').XastElement} element
* @returns {import('../lib/types.js').CSSDeclarationMap}
*/
function getInheritableProperties(element) {
/** @type {import('../lib/types.js').CSSDeclarationMap} */
const props = new Map();

// Gather all inheritable attributes.
for (const [k, v] of Object.entries(element.attributes)) {
const value = getSVGAttributeValue(v);
if (inheritableAttrs.has(k)) {
props.set(k, { value: svgToString(value), important: false });
} else if (k === 'transform') {
const cssValue = svgAttTransformToCSS(value);
if (cssValue) {
props.set(k, cssValue);
}
} else if (TRANSFORM_PROP_NAMES.includes(k)) {
props.set(k, { value: svgToString(value), important: false });
}
}

// Overwrite with inheritable properties.
const styleProps = getStyleDeclarations(element);
if (styleProps) {
styleProps.forEach((v, k) => {
if (inheritableAttrs.has(k) || TRANSFORM_PROP_NAMES.includes(k)) {
if (v === null) {
props.delete(k);
} else {
props.set(k, v);
}
}
});
}

return props;
}

/**
* @param {string|import('../lib/types.js').SVGAttValue} v
* @returns {import('../lib/types.js').SVGAttValue}
*/
function getSVGAttributeValue(v) {
if (typeof v === 'string') {
return { strVal: v };
}
return v;
}
97 changes: 63 additions & 34 deletions plugins/moveElemsStylesToGroup.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { cssToString, cssTransformToSVGAtt } from '../lib/css-parse-decl.js';
import { getStyleDeclarations } from '../lib/css-tools.js';
import { writeStyleAttribute } from '../lib/css.js';
import { inheritableAttrs } from './_collections.js';

/**
* @typedef {import('../lib/types.js').CSSDeclarationMap} CSSDeclarationMap
*/
import { svgToString } from '../lib/svg-parse-att.js';
import { getInheritableProperties, TRANSFORM_PROP_NAMES } from './_styles.js';

export const name = 'moveElemsStylesToGroup';
export const description =
Expand All @@ -26,92 +24,123 @@ export const fn = (root, params, info) => {
return {
element: {
exit: (node) => {
// Run on exit so children are processed first.

// Process only groups with more than 1 child.
if (node.name !== 'g' || node.children.length <= 1) {
return;
}

// Record child properties so we don't have to re-parse them.
/** @type {Map<import('../lib/types.js').XastElement,Map<string,{value:string,important?:boolean}>>} */
const childProperties = new Map();

/**
* Find common properties in group children.
* @type {CSSDeclarationMap}
* @type {import('../lib/types.js').CSSDeclarationMap}
*/
const commonProperties = new Map();
/** @type {Set<string>} */
const transformPropertiesFound = new Set();
let initial = true;

for (const child of node.children) {
if (child.type !== 'element') {
continue;
}

const properties = getStyleDeclarations(child);
if (properties === undefined) {
const childProperties = getInheritableProperties(child);
if (childProperties === undefined) {
return;
}
childProperties.set(child, properties);

if (initial) {
initial = false;
// Collect all inheritable properties from first child element.
for (const [name, value] of properties.entries()) {
// Consider only inheritable attributes and transform. Transform is not inheritable, but according
// to https://developer.mozilla.org/docs/Web/SVG/Element/g, "Transformations applied to the
// <g> element are performed on its child elements"
if (inheritableAttrs.has(name) || name === 'transform') {
commonProperties.set(name, value);
}
for (const [name, value] of childProperties.entries()) {
commonProperties.set(name, value);
}
} else {
// exclude uncommon attributes from initial list
for (const [name, value] of commonProperties) {
const dec = properties.get(name);
for (const [name, commonValue] of commonProperties) {
const childProperty = childProperties.get(name);
if (
!dec ||
dec.value !== value.value ||
dec.important !== value.important
!childProperty ||
cssToString(childProperty) !== cssToString(commonValue) ||
childProperty.important !== commonValue.important
) {
commonProperties.delete(name);
}
}
}

// Record any transform properties found.
TRANSFORM_PROP_NAMES.forEach((name) => {
if (childProperties.has(name)) {
transformPropertiesFound.add(name);
}
});

if (commonProperties.size === 0) {
return;
}
}

// Preserve transform on children when group has filter or clip-path or mask.
const groupOwnStyle = styleData.computeOwnStyle(node);

// Don't move transform on children when group has filter or clip-path or mask, or if not all transform properties can
// be moved.
let hasAllTransforms = true;
transformPropertiesFound.forEach((name) => {
if (!commonProperties.has(name)) {
hasAllTransforms = false;
}
});
if (
groupOwnStyle.has('clip-path') ||
groupOwnStyle.has('filter') ||
groupOwnStyle.has('mask')
groupOwnStyle.has('mask') ||
!hasAllTransforms
) {
commonProperties.delete('transform');
TRANSFORM_PROP_NAMES.forEach((name) => commonProperties.delete(name));
}

// Add common child properties to group.
/** @type {CSSDeclarationMap} */
/** @type {import('../lib/types.js').CSSDeclarationMap} */
const groupProperties = getStyleDeclarations(node) ?? new Map();

for (const [name, value] of commonProperties) {
groupProperties.set(name, value);
}

const cssTransform = groupProperties.get('transform');
if (cssTransform) {
// Make sure we can translate it to an attribute.
const attTransform = cssTransformToSVGAtt(cssTransform);
if (attTransform) {
// Add transform as an attribute.
groupProperties.delete('transform');
const currentTransform = node.attributes.transform ?? '';
node.attributes.transform =
currentTransform + svgToString(attTransform);
} else {
// This shouldn't happen unless there's a CSS transform which can't be converted to an attribute; don't
// move the property.
groupProperties.delete('transform');
}
}

writeStyleAttribute(node, groupProperties);

// Delete common properties from children.
for (const child of node.children) {
if (child.type === 'element') {
/** @type {CSSDeclarationMap} */
// @ts-ignore - properties should be defined because
const properties = childProperties.get(child);
const childProperties = getStyleDeclarations(child);
for (const [name] of commonProperties) {
properties.delete(name);
if (childProperties) {
childProperties.delete(name);
}
delete child.attributes[name];
}
if (childProperties) {
writeStyleAttribute(child, childProperties);
}
writeStyleAttribute(child, properties);
}
}
},
Expand Down
2 changes: 0 additions & 2 deletions plugins/preset-default.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import * as inlineStyles from './inlineStyles.js';
import * as minifyPathData from './minifyPathData.js';
import * as minifyStyles from './minifyStyles.js';
import * as minifyTransforms from './minifyTransforms.js';
import * as moveElemsAttrsToGroup from './moveElemsAttrsToGroup.js';
import * as moveElemsStylesToGroup from './moveElemsStylesToGroup.js';
import * as removeComments from './removeComments.js';
import * as removeDesc from './removeDesc.js';
Expand Down Expand Up @@ -51,7 +50,6 @@ const presetDefault = createPreset({
removeEmptyText,
minifyTransforms,
convertEllipseToCircle,
moveElemsAttrsToGroup,
moveElemsStylesToGroup,
collapseGroups,
convertShapeToPath,
Expand Down
2 changes: 0 additions & 2 deletions plugins/preset-next.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import * as inlineStyles from './inlineStyles.js';
import * as minifyPathData from './minifyPathData.js';
import * as minifyStyles from './minifyStyles.js';
import * as minifyTransforms from './minifyTransforms.js';
import * as moveElemsAttrsToGroup from './moveElemsAttrsToGroup.js';
import * as moveElemsStylesToGroup from './moveElemsStylesToGroup.js';
import * as removeComments from './removeComments.js';
import * as removeDesc from './removeDesc.js';
Expand Down Expand Up @@ -51,7 +50,6 @@ const presetNext = createPreset({
removeEmptyText,
minifyTransforms,
convertEllipseToCircle,
moveElemsAttrsToGroup,
moveElemsStylesToGroup,
collapseGroups,
convertShapeToPath,
Expand Down
18 changes: 18 additions & 0 deletions test/plugins/moveElemsStylesToGroup.12.svg.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Don't move transform if there is at least one transform-origin that can't be moved.
===

<svg xmlns="http://www.w3.org/2000/svg" viewBox="-10 -10 20 20">
<g>
<rect x="3" y="4" width="2" height="3" stroke="black" fill="red" transform-origin="3 4" transform="rotate(30)"/>
<rect x="3" y="4" width="2" height="3" stroke="black" fill="green" transform="rotate(30)"/>
</g>
</svg>

@@@

<svg xmlns="http://www.w3.org/2000/svg" viewBox="-10 -10 20 20">
<g style="stroke:black">
<rect x="3" y="4" width="2" height="3" fill="red" transform-origin="3 4" transform="rotate(30)"/>
<rect x="3" y="4" width="2" height="3" fill="green" transform="rotate(30)"/>
</g>
</svg>
18 changes: 18 additions & 0 deletions test/plugins/moveElemsStylesToGroup.13.svg.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Don't move transform-origin if there is at least one transform that can't be moved.
===

<svg xmlns="http://www.w3.org/2000/svg" viewBox="-10 -10 20 20">
<g>
<rect x="3" y="4" width="2" height="3" stroke="black" fill="red" transform-origin="3 4" transform="rotate(30)"/>
<rect x="3" y="4" width="2" height="3" stroke="black" fill="green" transform-origin="3 4" transform="rotate(130)"/>
</g>
</svg>

@@@

<svg xmlns="http://www.w3.org/2000/svg" viewBox="-10 -10 20 20">
<g style="stroke:black">
<rect x="3" y="4" width="2" height="3" fill="red" transform-origin="3 4" transform="rotate(30)"/>
<rect x="3" y="4" width="2" height="3" fill="green" transform-origin="3 4" transform="rotate(130)"/>
</g>
</svg>
Loading