Skip to content

feat(icons): Update icons.yml 0916#9595

Merged
kodiakhq[bot] merged 22 commits into
carbon-design-system:mainfrom
LMapes:patch-12
Oct 5, 2021
Merged

feat(icons): Update icons.yml 0916#9595
kodiakhq[bot] merged 22 commits into
carbon-design-system:mainfrom
LMapes:patch-12

Conversation

@LMapes

@LMapes LMapes commented Sep 3, 2021

Copy link
Copy Markdown
Contributor

Updated icons.yml with

aliases
friendly names

approved by BXD

Changelog

Changed

  • icons.yml

changes made correspond to issue #9473

@netlify

netlify Bot commented Sep 3, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 25fc1c5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/615c7507d1e23e000781c499

😎 Browse the preview: https://deploy-preview-9595--carbon-react-next.netlify.app

@netlify

netlify Bot commented Sep 3, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 25fc1c5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/615c750758ec0e0008ec0220

😎 Browse the preview: https://deploy-preview-9595--carbon-elements.netlify.app

@netlify

netlify Bot commented Sep 3, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 25fc1c5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/615c75075689a20007c89ffc

😎 Browse the preview: https://deploy-preview-9595--carbon-components-react.netlify.app

updated metadata
@LMapes LMapes marked this pull request as ready for review September 16, 2021 15:41
@LMapes LMapes requested a review from a team as a code owner September 16, 2021 15:41
@LMapes LMapes requested review from jnm2377 and joshblack September 16, 2021 15:41
@LMapes LMapes changed the title 0903Update icons.yml feat(icons): Update icons.yml 0916 Sep 16, 2021
@jnm2377

jnm2377 commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

Looks like some tests are failing for this.

@LMapes

LMapes commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

@jnm2377 I don't know how to fix these errors.

@jnm2377

jnm2377 commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

Looks like it's breaking bc of bad indentation. I would double check your formatting to make sure there aren't other bad indentations besides this one.
Screen Shot 2021-09-17 at 3 08 15 PM

Also, public tests might need to be updated. I'm not sure. But looking at the PR you linked, it definitely updated test after adding new icon data.
Screen Shot 2021-09-17 at 3 09 41 PM

Comment thread packages/icons/icons.yml Outdated
@LMapes

LMapes commented Sep 17, 2021

Copy link
Copy Markdown
Contributor Author

@joshblack I don't know how to fix indentation. It looked visually okay to me in VS Code.

Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
Comment thread packages/icons/icons.yml Outdated
@LMapes LMapes requested a review from a team as a code owner September 21, 2021 21:15
@mjabbink

mjabbink commented Oct 4, 2021

Copy link
Copy Markdown

@LMapes @jnm2377 Did the naming get resolved on this issue? I’m seeing lots of code names showing up in the library that I think this issue will resolve.

@LMapes

LMapes commented Oct 4, 2021

Copy link
Copy Markdown
Contributor Author

@mjabbink I added all the metadata content for all the UI icons into the .yml. Why it's not displaying by now is unclear to me.

@jnm2377

jnm2377 commented Oct 4, 2021

Copy link
Copy Markdown
Contributor

Hey @LMapes sorry if I didn't make it clear in the past comments. I brought up tests failing a while back, which need to be addressed before this PR can get merged. Typically, contributors are responsible for updating their PRs accordingly when we give feedback or leave reviews. I tried to help out by updating all of the indentation errors your code had, but looks like the snapshots still need to be updated since you've added new metadata.

Comment thread packages/icons/icons.yml Outdated
@LMapes

LMapes commented Oct 4, 2021

Copy link
Copy Markdown
Contributor Author

@mjabbink I don't know how to correct this. Please assign a Carbon dev.

@Katie-A-IBM

Copy link
Copy Markdown

@joshblack @jnm2377 @tay1orjones
Hi All, Would you be able to set up a call to discuss this issue?
Thanks
cc @LMapes

@tay1orjones

Copy link
Copy Markdown
Member

@LMapes I'm happy to fix it, but don't know what the proper values should be. This is what it is currently:

- name: map--identify
  - map
    - travel
    - point
    - identify
    - ID
    - center
    - locate
    - location
    - graph
  sizes:
    - 32

There's no friendlyName or aliases entries. They were there previously but were removed as part of this PR. Should it be this instead?

- name: map--identify
  friendly_name: map--identify
  aliases:
    - map
    - travel
    - point
    - identify
    - ID
    - center
    - locate
    - location
    - graph
  sizes:
    - 32

@LMapes

LMapes commented Oct 5, 2021

Copy link
Copy Markdown
Contributor Author

@tay1orjones Thanks for clarifying what you need. This should work

  • name: map--identify
    friendly_name: Map identify
    aliases:
    • map
    • travel
    • point
    • identify
    • target
    • exact
    • ID
    • center
    • locate
    • location
    • graph
      sizes:
    • 32

@tay1orjones tay1orjones left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've corrected the map--identify metadata as well as another piece of indentation I found. There were also a few entries that had the .svg suffix added, which isn't needed and caused the build to fail.

@kodiakhq kodiakhq Bot merged commit 61b5aaa into carbon-design-system:main Oct 5, 2021
@mjabbink

mjabbink commented Oct 7, 2021

Copy link
Copy Markdown

@LMapes I’m not sure if this issue covers all of these naming issues (below list) but these all need attention. The below names were copied directly from the library as they appear. The friendly names are missing for all of the below.

continue
continued--filled
fit-to-height
fit-to-width
settings--services
settings--view
cost
cost--total
document--signed
document--unprotected
document--protected
document--security
result--new
result--old
result--cancelled
rule--draft
rule--filled
rule--test
task--add
task--asset-view
task--approved
task--complete
task--location
task--settings
task--star
task--tools
ungroup-objects
group-objects--new
group-objects--save
window--black-saturation
image--search--alt
airplay--filled
asset--confirm
asset--digital-twin
asset--view
radio--combat
radio--push-to-talk
wifi--controller
phone--application
location--save
map--center
map--identify
dog-walker
user--access
user--military
user--service-desk
user--settings
carbon-accounting
energy--renewable
solar-panel
temperature--celsius
temperature--celsius--alt
temperature--fahrenheit
temperature--fahrenheit--alt
tropical-storm--tracks
wind-power
cell-tower
drone--delivery
drone--front
drone--video
cloud--offline

@LMapes

LMapes commented Oct 7, 2021

Copy link
Copy Markdown
Contributor Author

@mjabbink All of these were in the metadata yml file for this issue and should be appearing on screen whenever the IDL update happens

@mjabbink

mjabbink commented Oct 7, 2021

Copy link
Copy Markdown

@LMapes ok great, I was not sure all of these were included. thanks for the update.

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.

5 participants