fix(sketch): resolve sketch plugin color layer style duplication (#7762)

* refactor(sketch): use param object in syncColorStyles * refactor(colors): declare function before calling * refactor(sketch): use param object in formatSharedColorStyleName * refactor(sketch): use param object in syncColorStyle * fix(sketch): remove type param from formatSharedColorStyleName * fix(sketch): remove styleType param from syncThemeColorStyles * fix(sketch): remove type param from syncColorStyles and syncColorStyle * fix(shared): remove 'fill' from shared style name * fix(colors): remove style type from generate command * fix(sharedStyles): rename layers to match updated format Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
carbon-design-system · Feb 12, 2021 · 3d1c3e6 · 3d1c3e6
1 parent 82abdc6
commit 3d1c3e6
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 64 deletions.
diff --git a/packages/sketch/src/commands/colors/generate.js b/packages/sketch/src/commands/colors/generate.js
@@ -19,12 +19,12 @@ export function generate() {
   command('commands/colors/generate', () => {
     const document = Document.getSelectedDocument();
     const page = selectPage(findOrCreatePage(document, 'color'));
-    const sharedStyles = syncColorStyles(document, 'fill');
+    const sharedStyles = syncColorStyles({ document });
     const { black, white, colors, support } = groupByKey(
       sharedStyles,
       (sharedStyle) => {
         const { name } = sharedStyle;
-        const [_category, _type, swatch] = name.split(' / ');
+        const [_category, swatch] = name.split(' / ');
         switch (swatch) {
           case 'black':
             return 'black';
@@ -43,7 +43,7 @@ export function generate() {
     let Y_OFFSET = 0;
 
     const swatches = groupByKey(colors, (sharedStyle) => {
-      const [_category, _type, swatch] = sharedStyle.name.split('/');
+      const [_category, swatch] = sharedStyle.name.split('/');
       return swatch;
     });
 

diff --git a/packages/sketch/src/commands/colors/sync.js b/packages/sketch/src/commands/colors/sync.js
@@ -11,6 +11,6 @@ import { syncColorStyles } from '../../sharedStyles/colors';
 
 export function sync() {
   command('commands/colors/sync', () => {
-    syncColorStyles(Document.getSelectedDocument(), 'fill');
+    syncColorStyles({ document: Document.getSelectedDocument() });
   });
 }
diff --git a/packages/sketch/src/commands/icons/shared.js b/packages/sketch/src/commands/icons/shared.js
@@ -20,9 +20,9 @@ export function syncIconSymbols({
   symbolsPage,
   sizes = [32, 24, 20, 16],
 }) {
-  const sharedStyles = syncColorStyles(document, 'fill');
+  const sharedStyles = syncColorStyles({ document });
   const [sharedStyle] = sharedStyles.filter(
-    ({ name }) => name === 'color / fill / black'
+    ({ name }) => name === 'color / black'
   );
 
   if (!sharedStyle) {

diff --git a/packages/sketch/src/commands/test/sync-shared-styles.js b/packages/sketch/src/commands/test/sync-shared-styles.js
@@ -42,13 +42,17 @@ export function testSyncSharedStyles() {
     /**
      * Testing shared layer styles
      */
-    const sharedStyle = syncColorStyle(document, 'black', '#000000', 'fill');
+    const sharedStyle = syncColorStyle({
+      document,
+      name: 'black',
+      value: '#000000',
+    });
 
     if (document.sharedLayerStyles.length !== 1) {
       throw new Error('Expected sync command to generate a shared layer style');
     }
 
-    syncColorStyle(document, 'black', '#000000', 'fill');
+    syncColorStyle({ document, name: 'black', value: '#000000' });
 
     if (document.sharedLayerStyles.length !== 1) {
       throw new Error(
@@ -118,7 +122,7 @@ export function testSyncSharedStyles() {
       throw new Error('The layer is not in sync with the shared style');
     }
 
-    syncColorStyle(document, 'black', '#dedede', 'fill');
+    syncColorStyle({ document, name: 'black', value: '#dedede' });
 
     if (getLayerFillColor() !== '#dededeff') {
       throw new Error('The layer did not update to the new shared style');

diff --git a/packages/sketch/src/commands/themes/generate.js b/packages/sketch/src/commands/themes/generate.js
@@ -21,7 +21,7 @@ export function generate() {
   command('commands/themes/generate', () => {
     const document = Document.getSelectedDocument();
     const page = selectPage(findOrCreatePage(document, 'themes'));
-    const sharedStyles = syncThemeColorStyles(document, 'fill');
+    const sharedStyles = syncThemeColorStyles(document);
 
     const tokens = groupByKey(sharedStyles, (sharedStyle) => {
       const [_namespace, _category, _group, token] = sharedStyle.name.split(

diff --git a/packages/sketch/src/commands/themes/sync.js b/packages/sketch/src/commands/themes/sync.js
@@ -12,6 +12,6 @@ import { syncThemeColorStyles } from '../../sharedStyles/themes';
 export function sync() {
   command('commands/themes/sync', () => {
     const document = Document.getSelectedDocument();
-    syncThemeColorStyles(document, 'fill');
+    syncThemeColorStyles(document);
   });
 }
diff --git a/packages/sketch/src/sharedStyles/colors.js b/packages/sketch/src/sharedStyles/colors.js
@@ -13,21 +13,35 @@ import { syncColorStyle } from '../tools/sharedStyles';
 // that we need to render
 const { black, white, orange, yellow, ...swatches } = colors;
 
+/**
+ * Our shared style name will need to have the `color` namespace alongside a
+ * name for the swatch, the style type, and an optional grade.
+ * @param {object} params - formatSharedColorStyleName parameters
+ * @param {string} params.name
+ * @param {string?} params.grade
+ * @returns {string}
+ */
+function formatSharedColorStyleName({ name, grade }) {
+  return ['color', name.split('-').join(' '), grade]
+    .filter(Boolean)
+    .join(' / ');
+}
+
 /**
  * Sync color shared styles to the given document and return the result
- * @param {Document} document
+ * @param {object} params - syncColorStyles parameters
+ * @param {Document} params.document
  * @returns {Array<SharedStyle>}
  */
-export function syncColorStyles(document, type) {
+export function syncColorStyles({ document }) {
   const sharedStyles = Object.keys(swatches).flatMap((swatchName) => {
     const name = formatTokenName(swatchName);
     const result = Object.keys(swatches[swatchName]).map((grade) => {
-      return syncColorStyle(
+      return syncColorStyle({
         document,
-        formatSharedStyleName(name, type, grade),
-        swatches[swatchName][grade],
-        type
-      );
+        name: formatSharedColorStyleName({ name, grade }),
+        value: swatches[swatchName][grade],
+      });
     });
     return result;
   });
@@ -38,27 +52,12 @@ export function syncColorStyles(document, type) {
     ['orange', orange['40']],
     ['yellow', yellow['30']],
   ].map(([name, value]) => {
-    return syncColorStyle(
+    return syncColorStyle({
       document,
-      formatSharedStyleName(name, type),
+      name: formatSharedColorStyleName({ name }),
       value,
-      type
-    );
+    });
   });
 
   return sharedStyles.concat(singleColors);
 }
-
-/**
- * Our shared style name will need to have the `color` namespace alongside a
- * name for the swatch, the style type, and an optional grade.
- * @param {string} name
- * @param {string} type
- * @param {string?} grade
- * @returns {string}
- */
-function formatSharedStyleName(name, type, grade) {
-  return ['color', type, name.split('-').join(' '), grade]
-    .filter(Boolean)
-    .join(' / ');
-}
diff --git a/packages/sketch/src/sharedStyles/themes.js b/packages/sketch/src/sharedStyles/themes.js
@@ -21,10 +21,9 @@ const { colors } = tokens;
 /**
  * Sync theme color shared styles to the given document and return the result
  * @param {Document} document
- * @param {string} styleType
  * @returns {Array<SharedStyle>}
  */
-export function syncThemeColorStyles(document, styleType) {
+export function syncThemeColorStyles(document) {
   const themes = {
     'White theme': white,
     'Gray 10 theme': g10,
@@ -44,7 +43,11 @@ export function syncThemeColorStyles(document, styleType) {
         const name = `theme / ${theme.toLowerCase()} / ${type} tokens / ${formatTokenName(
           token
         )}`;
-        return syncColorStyle(document, name, themes[theme][token], styleType);
+        return syncColorStyle({
+          document,
+          name,
+          value: themes[theme][token],
+        });
       });
   });
 

diff --git a/packages/sketch/src/tools/sharedStyles.js b/packages/sketch/src/tools/sharedStyles.js
@@ -29,6 +29,21 @@ export function syncSharedStyle(
       : document.sharedTextStyles;
   const [sharedStyle] = Array.from(documentSharedStyles).filter(
     (sharedStyle) => {
+      /**
+       * TODO: remove the following block after next Sketch plugin release
+       * backwards compatibility to avoid breaking changes from #5664, #5744
+       * we search for style names with the following format
+       *   `color/teal/60`
+       * and reformat it to
+       *   `color / teal / 60`
+       * this search and replace will not be needed after the plugin has been
+       * published with renamed style layers
+       */
+      // start removal
+      if (sharedStyle.name.split('/').join(' / ') === name) {
+        sharedStyle.name = name;
+      }
+      // end removal
       return sharedStyle.name === name;
     }
   );
@@ -62,31 +77,19 @@ export function syncSharedStyle(
 
 /**
  * Sync the given color value as a shared style for the document
- * @param {Document} document
- * @param {string} name
- * @param {string} value
- * @param {string} type
+ * @param {object} params - syncColorStyle parameters
+ * @param {Document} params.document
+ * @param {string} params.name
+ * @param {string} params.value
  * @returns {SharedStyle}
  */
-export function syncColorStyle(document, name, value, type) {
-  if (type === 'fill') {
-    return syncSharedStyle(document, name, {
-      fills: [
-        {
-          color: value,
-          fillType: Style.FillType.Color,
-        },
-      ],
-    });
-  }
-  if (type === 'border') {
-    return syncSharedStyle(document, name, {
-      borders: [
-        {
-          color: value,
-          fillType: Style.FillType.Color,
-        },
-      ],
-    });
-  }
+export function syncColorStyle({ document, name, value }) {
+  return syncSharedStyle(document, name, {
+    fills: [
+      {
+        color: value,
+        fillType: Style.FillType.Color,
+      },
+    ],
+  });
 }