From d7cddd855fe4ced710643ee2cf1aeeeeb371d4c1 Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:21:13 -0600 Subject: [PATCH 1/6] refactor(symbols): remove syncSymbolLayers --- packages/sketch/src/tools/symbols.js | 74 +--------------------------- 1 file changed, 1 insertion(+), 73 deletions(-) diff --git a/packages/sketch/src/tools/symbols.js b/packages/sketch/src/tools/symbols.js index 752d92afb690..cc38821df27e 100644 --- a/packages/sketch/src/tools/symbols.js +++ b/packages/sketch/src/tools/symbols.js @@ -27,84 +27,12 @@ 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']); /** From d7ab22270149e3e626348b46d5f0487c19a3f1eb Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:21:56 -0600 Subject: [PATCH 2/6] refactor(symbols): remove merge function --- packages/sketch/src/tools/symbols.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/packages/sketch/src/tools/symbols.js b/packages/sketch/src/tools/symbols.js index cc38821df27e..4ff0a04268c5 100644 --- a/packages/sketch/src/tools/symbols.js +++ b/packages/sketch/src/tools/symbols.js @@ -32,23 +32,3 @@ export function syncSymbol(symbols, sharedLayerStyles, name, config) { return symbol; } - -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]; - } - } -} From 783ea3f67c8b8d7b338041222e52d9e696be534c Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:26:42 -0600 Subject: [PATCH 3/6] refactor(symbols): use param object instead of individual params --- packages/sketch/src/commands/icons/shared.js | 15 ++++++++++----- .../sketch/src/commands/test/sync-symbol-id.js | 14 +++++++------- packages/sketch/src/tools/symbols.js | 12 +++++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js index c1ca538565d9..be11b10dcea2 100644 --- a/packages/sketch/src/commands/icons/shared.js +++ b/packages/sketch/src/commands/icons/shared.js @@ -40,12 +40,17 @@ export function syncIconSymbols( ); return artboards.map((artboard) => { - return syncSymbol(symbols, sharedLayerStyles, artboard.name, { + return syncSymbol({ + symbols, + sharedLayerStyles, 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/test/sync-symbol-id.js b/packages/sketch/src/commands/test/sync-symbol-id.js index 5ae53c3956ce..e9f06cb8e2f3 100644 --- a/packages/sketch/src/commands/test/sync-symbol-id.js +++ b/packages/sketch/src/commands/test/sync-symbol-id.js @@ -26,11 +26,11 @@ export function testSyncSymbolId() { command('commands/test/sync-symbol-id', () => { const document = Document.getSelectedDocument(); - syncSymbol( - document.getSymbols(), - document.sharedLayerStyles, - 'test-symbol', - { + syncSymbol({ + symbols: document.getSymbols(), + sharedLayerStyles: document.sharedLayerStyles, + name: 'test-symbol', + config: { layers: [ new ShapePath({ name: 'Inner', @@ -38,7 +38,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 4ff0a04268c5..7df47c390243 100644 --- a/packages/sketch/src/tools/symbols.js +++ b/packages/sketch/src/tools/symbols.js @@ -10,13 +10,15 @@ 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 {Array} params.sharedLayerStyles + * @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, sharedLayerStyles, name, config }) { const symbol = symbols.find((symbol) => symbol.name === name); if (!symbol) { From 678e22a66aa7efb9794f49d5b043ab5b2bf22f6b Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:28:57 -0600 Subject: [PATCH 4/6] refactor(symbols): remove unused param from syncSymbol --- packages/sketch/src/commands/icons/shared.js | 1 - packages/sketch/src/commands/test/sync-symbol-id.js | 1 - packages/sketch/src/tools/symbols.js | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js index be11b10dcea2..e5a148785013 100644 --- a/packages/sketch/src/commands/icons/shared.js +++ b/packages/sketch/src/commands/icons/shared.js @@ -42,7 +42,6 @@ export function syncIconSymbols( return artboards.map((artboard) => { return syncSymbol({ symbols, - sharedLayerStyles, name: artboard.name, config: { name: artboard.name, diff --git a/packages/sketch/src/commands/test/sync-symbol-id.js b/packages/sketch/src/commands/test/sync-symbol-id.js index e9f06cb8e2f3..5e1b694e467b 100644 --- a/packages/sketch/src/commands/test/sync-symbol-id.js +++ b/packages/sketch/src/commands/test/sync-symbol-id.js @@ -28,7 +28,6 @@ export function testSyncSymbolId() { syncSymbol({ symbols: document.getSymbols(), - sharedLayerStyles: document.sharedLayerStyles, name: 'test-symbol', config: { layers: [ diff --git a/packages/sketch/src/tools/symbols.js b/packages/sketch/src/tools/symbols.js index 7df47c390243..6e5536b5cf8d 100644 --- a/packages/sketch/src/tools/symbols.js +++ b/packages/sketch/src/tools/symbols.js @@ -12,13 +12,12 @@ import { SymbolMaster } from 'sketch/dom'; * return a new symbol if none exist with the given name. * @param {object} params - syncSymbol parameters * @param {Array} params.symbols - * @param {Array} params.sharedLayerStyles * @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) { From 4732d9dcda95e67129507a642d938663d60a842a Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:32:44 -0600 Subject: [PATCH 5/6] refactor(shared): use param obj instead of individual params in syncIconSymbols --- packages/sketch/src/commands/icons/shared.js | 6 +++--- packages/sketch/src/commands/icons/sync.js | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js index e5a148785013..b7304f344e99 100644 --- a/packages/sketch/src/commands/icons/shared.js +++ b/packages/sketch/src/commands/icons/shared.js @@ -14,13 +14,13 @@ 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' diff --git a/packages/sketch/src/commands/icons/sync.js b/packages/sketch/src/commands/icons/sync.js index 02230a8d350c..8222c15bfbb7 100644 --- a/packages/sketch/src/commands/icons/sync.js +++ b/packages/sketch/src/commands/icons/sync.js @@ -15,13 +15,13 @@ 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] - ); + sharedLayerStyles: document.sharedLayerStyles, + sizes: [16, 20], + }); }); } @@ -30,12 +30,12 @@ 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] - ); + sharedLayerStyles: document.sharedLayerStyles, + sizes: [24, 32], + }); }); } From 5a2a0f0c4beba93d13f6cfedb310d6676b1aa5b7 Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Feb 2021 14:33:22 -0600 Subject: [PATCH 6/6] refactor(shared): remove unused param from `syncIconSymbols` --- packages/sketch/src/commands/icons/shared.js | 1 - packages/sketch/src/commands/icons/sync.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js index b7304f344e99..8f63c2297eed 100644 --- a/packages/sketch/src/commands/icons/shared.js +++ b/packages/sketch/src/commands/icons/shared.js @@ -18,7 +18,6 @@ export function syncIconSymbols({ document, symbols, symbolsPage, - sharedLayerStyles, sizes = [32, 24, 20, 16], }) { const sharedStyles = syncColorStyles(document, 'fill'); diff --git a/packages/sketch/src/commands/icons/sync.js b/packages/sketch/src/commands/icons/sync.js index 8222c15bfbb7..ea3d9c43e58d 100644 --- a/packages/sketch/src/commands/icons/sync.js +++ b/packages/sketch/src/commands/icons/sync.js @@ -19,7 +19,6 @@ export function syncSmallIcons() { document, symbols: Array.from(symbols), symbolsPage, - sharedLayerStyles: document.sharedLayerStyles, sizes: [16, 20], }); }); @@ -34,7 +33,6 @@ export function syncLargeIcons() { document, symbols: Array.from(symbols), symbolsPage, - sharedLayerStyles: document.sharedLayerStyles, sizes: [24, 32], }); });