diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js index c1ca538565d9..8f63c2297eed 100644 --- a/packages/sketch/src/commands/icons/shared.js +++ b/packages/sketch/src/commands/icons/shared.js @@ -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' @@ -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, + }, }); }); } diff --git a/packages/sketch/src/commands/icons/sync.js b/packages/sketch/src/commands/icons/sync.js index 02230a8d350c..ea3d9c43e58d 100644 --- a/packages/sketch/src/commands/icons/sync.js +++ b/packages/sketch/src/commands/icons/sync.js @@ -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], + }); }); } @@ -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], + }); }); } diff --git a/packages/sketch/src/commands/test/sync-symbol-id.js b/packages/sketch/src/commands/test/sync-symbol-id.js index 5ae53c3956ce..5e1b694e467b 100644 --- a/packages/sketch/src/commands/test/sync-symbol-id.js +++ b/packages/sketch/src/commands/test/sync-symbol-id.js @@ -26,11 +26,10 @@ 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', @@ -38,7 +37,7 @@ export function testSyncSymbolId() { frame: new Rectangle(0, 0, 16, 16), }), ], - } - ); + }, + }); }); } diff --git a/packages/sketch/src/tools/symbols.js b/packages/sketch/src/tools/symbols.js index 752d92afb690..6e5536b5cf8d 100644 --- a/packages/sketch/src/tools/symbols.js +++ b/packages/sketch/src/tools/symbols.js @@ -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} symbols - * @param {Array} 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} 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) { @@ -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} 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]; - } - } -}