Skip to content

Commit

Permalink
fix(sketch): resolve icon sync artwork issues (#7711)
Browse files Browse the repository at this point in the history 
* refactor(symbols): remove syncSymbolLayers

* refactor(symbols): remove merge function

* refactor(symbols): use param object instead of individual params

* refactor(symbols): remove unused param from syncSymbol

* refactor(shared): use param obj instead of individual params in syncIconSymbols

* refactor(shared): remove unused param from `syncIconSymbols`

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
@emyarod @andreancardona @kodiakhq
3 people authored Feb 3, 2021
1 parent f85dcd1 commit 3ce64f1
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 124 deletions.
21 changes: 12 additions & 9 deletions packages/sketch/src/commands/icons/shared.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ import { syncSymbol } from '../../tools/symbols';

const metadata = require('../../../generated/icons/metadata.json');

export function syncIconSymbols(
export function syncIconSymbols({
document,
symbols,
symbolsPage,
sharedLayerStyles,
sizes = [32, 24, 20, 16]
) {
sizes = [32, 24, 20, 16],
}) {
const sharedStyles = syncColorStyles(document, 'fill');
const [sharedStyle] = sharedStyles.filter(
({ name }) => name === 'color / fill / black'
Expand All @@ -40,12 +39,16 @@ export function syncIconSymbols(
);

return artboards.map((artboard) => {
return syncSymbol(symbols, sharedLayerStyles, artboard.name, {
return syncSymbol({
symbols,
name: artboard.name,
frame: artboard.frame,
layers: artboard.layers,
background: artboard.background,
parent: symbolsPage,
config: {
name: artboard.name,
frame: artboard.frame,
layers: artboard.layers,
background: artboard.background,
parent: symbolsPage,
},
});
});
}
Expand Down
18 changes: 8 additions & 10 deletions packages/sketch/src/commands/icons/sync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ export function syncSmallIcons() {
const document = Document.getSelectedDocument();
const symbolsPage = findOrCreateSymbolPage(document);
const symbols = document.getSymbols();
syncIconSymbols(
syncIconSymbols({
document,
Array.from(symbols),
symbols: Array.from(symbols),
symbolsPage,
document.sharedLayerStyles,
[16, 20]
);
sizes: [16, 20],
});
});
}

Expand All @@ -30,12 +29,11 @@ export function syncLargeIcons() {
const document = Document.getSelectedDocument();
const symbolsPage = findOrCreateSymbolPage(document);
const symbols = document.getSymbols();
syncIconSymbols(
syncIconSymbols({
document,
Array.from(symbols),
symbols: Array.from(symbols),
symbolsPage,
document.sharedLayerStyles,
[24, 32]
);
sizes: [24, 32],
});
});
}
13 changes: 6 additions & 7 deletions packages/sketch/src/commands/test/sync-symbol-id.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,18 @@ export function testSyncSymbolId() {
command('commands/test/sync-symbol-id', () => {
const document = Document.getSelectedDocument();

syncSymbol(
document.getSymbols(),
document.sharedLayerStyles,
'test-symbol',
{
syncSymbol({
symbols: document.getSymbols(),
name: 'test-symbol',
config: {
layers: [
new ShapePath({
name: 'Inner',
shapeType: ShapePath.ShapeType.Oval,
frame: new Rectangle(0, 0, 16, 16),
}),
],
}
);
},
});
});
}
105 changes: 7 additions & 98 deletions packages/sketch/src/tools/symbols.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import { SymbolMaster } from 'sketch/dom';
/**
* Sync the given symbol name with a corresponding config in a document. Will
* return a new symbol if none exist with the given name.
* @param {Array<SymbolMaster>} symbols
* @param {Array<SharedLayerStyles>} sharedLayerStyles
* @param {string} name - the name of the symbol
* @param {object} config - the config for the corresponding symbol master
* @param {object} params - syncSymbol parameters
* @param {Array<SymbolMaster>} params.symbols
* @param {string} params.name - the name of the symbol
* @param {object} params.config - the config for the corresponding symbol
* master
* @returns {SketchSymbol}
*/
export function syncSymbol(symbols, sharedLayerStyles, name, config) {
export function syncSymbol({ symbols, name, config }) {
const symbol = symbols.find((symbol) => symbol.name === name);

if (!symbol) {
Expand All @@ -27,100 +28,8 @@ export function syncSymbol(symbols, sharedLayerStyles, name, config) {
}

Object.keys(config).forEach((key) => {
if (key === 'layers') {
syncSymbolLayers(sharedLayerStyles, symbol, config);
} else {
symbol[key] = config[key];
}
symbol[key] = config[key];
});

return symbol;
}

/**
* Sync the layers between the original sketch symbol master and a proposed
* configuration for this symbol. We will try our best to update the layers for
* the original symbol, but may have to de-opt and use the proposed layers if we
* cannot safely merge the two.
* @param {Array<SharedLayerStyle>} sharedLayerStyles
* @param {SketchSymbol} original - the original sketch symbol master
* @param {object} proposed - a proposed set of changes to the symbol
* @returns {void}
*/
function syncSymbolLayers(sharedLayerStyles, original, proposed) {
// We're going to update the `layers` value on `original` to what is found on
// `proposed`. If we can, we'll update the `original` layer in-place. In cases
// where we cannot, we'll use the proposed layers.
//
// The preference is to keep the original layer in-place, namely for symbol
// overrides that may exist in the document. If we remove the layer and update
// it with a proposed layer, then we lose that override information meaning
// that designers will have to go through and update their symbols again
// instead of it automatically being updated.
//
// If nothing is shared between original and proposed, then the layers in
// proposed will be used.
original.layers = proposed.layers.map((proposedLayer) => {
// We often name nested layers that can be overridden.
const { name } = proposedLayer;
const originalLayer = original.layers.find((layer) => {
return layer.name === name;
});

// If we can find a layer in `proposed` with a corresponding `original`
// layer, then we'll attempt to merge the two.
if (originalLayer) {
// We can't safely merge if the type or shapeType have diverged so we'll
// need to default to the changed layer
if (
originalLayer.type !== proposedLayer.type ||
originalLayer.shapeType !== proposedLayer.shapeType
) {
return proposedLayer;
}

// Attempt to merge the two layers
merge(originalLayer, proposedLayer);

// If our original layer has a shared style, we'll update it to match
if (originalLayer.sharedStyleId) {
const sharedStyle = sharedLayerStyles.find((sharedStyle) => {
return originalLayer.sharedStyleId === sharedStyle.id;
});

if (sharedStyle) {
const opacity = originalLayer.style.opacity;
originalLayer.style.syncWithSharedStyle(sharedStyle);
// We'll want to keep things like opacity from being overridden by the
// shared style as we use it to hide layers in certain icons
originalLayer.style.opacity = opacity;
}
}

return originalLayer;
}

// By default, we'll defer to the proposed layer
return proposedLayer;
});
}

const defaultPropertyList = new Set(['frame', 'points', 'style']);

/**
* Merge a given source layer with a target layer. Optionally specify a
* propertyList that will specify what keys are safe to merge
*
* @param {object} source - the source layer that we are trying to update
* in-place
* @param {object} target - the target layer that contains the latest changes
* @param {?Set} propertyList - specify which properties are safe to merge
* @returns {void}
*/
function merge(source, target, propertyList = defaultPropertyList) {
for (let key in target) {
if (propertyList.has(key)) {
source[key] = target[key];
}
}
}

0 comments on commit 3ce64f1

Please sign in to comment.