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: add pictograms #389

Merged
merged 13 commits into from
Sep 26, 2019
Merged

feat: add pictograms #389

merged 13 commits into from
Sep 26, 2019

Conversation

vpicone
Copy link
Contributor

@vpicone vpicone commented Sep 24, 2019

Closes #388

  • Added Pictograms Library, Usage and Contribution
  • Renamed Iconography to Icons to match Pictograms and IDL
  • Moved the Library page to be the first tab when the sidebar item is clicked

@vercel
Copy link

vercel bot commented Sep 24, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://carbon-website-git-fork-vpicone-add-pictograms.carbon-design-system.now.sh

@connor-leech
Copy link
Contributor

The regex is causing each title to be sentence case even though a fair number of them contain acronyms or capital letters in the second word.

Examples:

  • "IBM z"
  • Api

@vpicone
Copy link
Contributor Author

vpicone commented Sep 25, 2019

@connor-leech the page uses the friendly_name from the metadata.yml property in the pictograms manifest on the carbon repo. We don't manipulate the friendly name at all, so you'll wanna open a PR to update the mis-capitilized stuff in that file.

@connor-leech
Copy link
Contributor

@vpicone My bad. I'll take a look a that.

@connor-leech
Copy link
Contributor

@vpicone Fixing that yml will be pretty time-consuming and I likely won't have time to do it this week. I'll open an issue, but don't let that block you for now.

@vpicone
Copy link
Contributor Author

vpicone commented Sep 25, 2019

Yeah I think they were written by hand so will likely need to be updated by hand as well.

@vercel vercel bot temporarily deployed to staging September 25, 2019 23:21 Inactive
@ghost ghost requested review from asudoh and removed request for a team September 25, 2019 23:28
@vpicone vpicone changed the title feat: add pictograms page feat: add pictograms Sep 25, 2019
@vpicone vpicone requested review from jeanservaas and laurenmrice and removed request for asudoh September 25, 2019 23:45
@vpicone
Copy link
Contributor Author

vpicone commented Sep 26, 2019

@laurenmrice that's looks like it's a bug from us processing the svgs in Carbon. Not sure if we want to block this until it gets fixed. Opened an issue here: carbon-design-system/carbon#4106

resolved here carbon-design-system/carbon#4107

@vpicone
Copy link
Contributor Author

vpicone commented Sep 26, 2019

@laurenmrice @chrisconnors-ibm both issues have been resolved

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

On the Contribute tab:
The following links don’t work and should be redirected to:
IBM Design Language visual style - https://www.ibm.com/design/language/elements/pictograms/design
IBM Design Sketch Kit - sketch://add-library/cloud/75VZZ

@vpicone
Copy link
Contributor Author

vpicone commented Sep 26, 2019

@laurenmrice fixed

Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

Looks good to the best of my knowledge!

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

I think the picto ‘Petri cultures’ should be under the category 'Life Science' not 'Life Sciences'? It is currently under Life Sciences but that seems to be a category duplicate, but just plural ? @chrisconnors-ibm to clarify.

@vpicone
Copy link
Contributor Author

vpicone commented Sep 26, 2019

@laurenmrice that’s an issue with the metadata upstream in carbon. Can you make an issue there? There’s other issues with the metadata capitalization @connor-leech has opened an issue to address.

@vpicone vpicone requested a review from laurenmrice September 26, 2019 19:13
Copy link
Contributor

@chrisconnors-ibm chrisconnors-ibm left a comment

Choose a reason for hiding this comment

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

Library page looks good; no errors, no undefined, search works as expected .

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Ok, I will make a separate issue for that and will approve cuz this looks good!

@vpicone vpicone merged commit e57609e into carbon-design-system:master Sep 26, 2019
@vpicone vpicone deleted the add-pictograms branch September 26, 2019 19:18
@chrisconnors-ibm
Copy link
Contributor

@laurenmrice you're exactly right. If @connor-leech is making other metadata changes in another issue, that'd be a swell place to correct Life Sciences should be Life Science.

The risks of managing directly yaml values that are supposed to be generated from categories.yml

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.

Add pictograms library page
6 participants