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

fix(sketch): add undefined exception handler in icon generate script #5583

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Mar 10, 2020

Closes #5582

This PR avoid pushing undefined as a Layer object to Sketch in the icon page generation script and adds an npm script for viewing the development log

Changelog

New

  • log npm script for viewing the development log

Changed

  • undefined exception handler in icons/generate script

Testing / Reviewing

  1. modify carbon/packages/sketch/src/commands/icons/shared.js to only deal with a subset of icons (either modify the start and end variables to be a subset of the full icons list or directly change the slice size in symbolsToSync)
  2. yarn build
  3. sync and generate the icons in Sketch

@emyarod emyarod requested a review from joshblack March 10, 2020 15:14
@emyarod emyarod requested a review from a team as a code owner March 10, 2020 15:14
@ghost ghost requested a review from asudoh March 10, 2020 15:14
@netlify
Copy link

netlify bot commented Mar 10, 2020

Deploy preview for carbon-components-react ready!

Built with commit a3d46e7

https://deploy-preview-5583--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Mar 10, 2020

Deploy preview for carbon-elements ready!

Built with commit a3d46e7

https://deploy-preview-5583--carbon-elements.netlify.com

@@ -68,18 +68,19 @@ export function generate() {
const [_type, _category, _subcategory, name, size] = parts;
return name === icon && size === '32';
});
const instance = symbol.createNewInstance();
instance.frame.offset(ICON_X_OFFSET, ICON_Y_OFFSET);
const instance = symbol?.createNewInstance();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wild, I think we might want to fail if the symbol doesn't exist. Maybe something like:

if (!symbol) {
  throw new Error(`Unable to find symbol for icon ${icon}`);
}

@emyarod emyarod force-pushed the 5582-sketch-icon-generate-script-undefined-exception-handler branch 2 times, most recently from 5733a36 to edb3925 Compare March 10, 2020 16:58
@emyarod emyarod requested a review from joshblack March 11, 2020 17:56
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 - Thanks @emyarod!

@emyarod emyarod force-pushed the 5582-sketch-icon-generate-script-undefined-exception-handler branch 2 times, most recently from b6e0784 to 3a7a807 Compare March 19, 2020 16:13
@emyarod emyarod force-pushed the 5582-sketch-icon-generate-script-undefined-exception-handler branch from 3a7a807 to a3d46e7 Compare March 19, 2020 16:14
@emyarod emyarod merged commit a301044 into carbon-design-system:master Mar 19, 2020
@emyarod emyarod deleted the 5582-sketch-icon-generate-script-undefined-exception-handler branch March 19, 2020 17:35
renmaddox pushed a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
…arbon-design-system#5583)

* chore: add log script

* chore: fix typo

* fix(sketch): handle undefined exception
renmaddox added a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
renmaddox added a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
renmaddox pushed a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
…arbon-design-system#5583)

* chore: add log script

* chore: fix typo

* fix(sketch): handle undefined exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sketch icon generate script cannot handle icon range subsets
3 participants