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

feat(pictograms): add four new pictograms to education category #6353

Merged

Conversation

dudley-ibm
Copy link
Contributor

Closes #

{{closes enterprise issue #6 add 4 pictograms}}

Changelog

New

  • {{focus.svg
    goals.svg
    growth.svg
    insights.svg}}

@dudley-ibm dudley-ibm requested a review from a team as a code owner June 26, 2020 17:59
@ghost ghost requested review from emyarod and joshblack June 26, 2020 17:59
@netlify
Copy link

netlify bot commented Jun 26, 2020

Deploy preview for carbon-elements ready!

Built with commit 311d134

https://deploy-preview-6353--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jun 26, 2020

Deploy preview for carbon-components-react ready!

Built with commit 311d134

https://deploy-preview-6353--carbon-components-react.netlify.app

@dudley-ibm dudley-ibm requested a review from a team as a code owner June 26, 2020 20:40
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

updated snapshots, looks good to me once CI passes

@emyarod emyarod force-pushed the add-four-education-pictograms branch from f0263de to 6a37184 Compare June 26, 2020 21:25
@dudley-ibm
Copy link
Contributor Author

@tw15egan @joshblack after I created this PR, I was asked to change the name of one of the SVGs. The previously submitted focus.svg needs to be feedback--02.svg-- this would require me to remove focus.svg because that SVG has layers in it that also say focus. What's the easiest way to make this swap at this stage of the pull request?

@joshblack joshblack changed the title added four new pictograms to education category feat(pictograms): add four new pictograms to education category Jun 29, 2020
@joshblack
Copy link
Contributor

Hey @dudley-ibm! 👋 Feel free to rename focus.svg to whatever the preferred name would be! Since you are adding this in this PR, and it's not in the library yet, there's also no need to worry about the deprecation flow and you can just make the changes directly.

@tw15egan
Copy link
Member

tw15egan commented Jul 2, 2020

@dudley-ibm I've goen ahead and updated focus to feedback--02. This included:

  • Updating pictograms.yml
  • Updating categories.yml
  • Changing the svg name in the src directory, as well as editing the id of the actual svg element

Let me know if this is what you had intended to change 👍

@dudley-ibm
Copy link
Contributor Author

Thanks @tw15egan! Here's what I was trying to accomplish:

  1. Downloadable file from IDL should be the asset I provide. For a while @joshblack made that happen but in this last release of pictograms, that is not the case (attachment, see pasteboard size difference/layer naming).
  2. In submitted SVG, the artwork lives on a layer that has a matching name to the file itself. Wasn't sure how to accomplish that through the UI without flat out replacing the file.
  3. All this may be moot with the upcoming replacement files that will be outlined (not stroked) and a 32x32px paste
    board instead of 48x48px.

svg_carbon_discrepancy

@tw15egan tw15egan merged commit 4e32b75 into carbon-design-system:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants