feat(icons): modified and added icons

[ArtBlue]

Fixes #2381

• This PR contains CSS changes
• This PR does not contain CSS changes

Description

• Added new icons
• Modified some current icons

Notes

Maybe it's because of the switchover to Marko Run, and the fact that we're now using a Marko component (master-icons.marko), but there was an issue with the icons appearing in the docs. The icons were being added to icons.svg, but they were not being added to master-icons.marko. I don't know where the link is broken. I looked into it, but couldn't figure it out. It seemed like npm run copy:svgToDocs was not being triggered, so I ran it, and it worked. I added this step as an optional thing to do with icon creation if icons don't show up.

[changeset-bot]

🦋 Changeset detected

Latest commit: 2e93849

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@ebay/skin	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[agliga]

LGTM.
That said, I believe we can change the command to be run automatically and don't need to document it.

[ArtBlue]

LGTM. That said, I believe we can change the command to be run automatically and don't need to document it.

@agliga , it took me a while to figure out what in the world was going on, which increased the scope of this issue (was supposed to be a size 1). Looking at the entire mechanism, I couldn't find a quick and intuitive solution. Instead of trial and error, which would have taken even more time, I opted for this in the short-term.

I'm good adding the script in, removing the docs, and avoiding the tech debt. Just suggest a change in the PR and I'll merge it.

[agliga]

LGTM. That said, I believe we can change the command to be run automatically and don't need to document it.

@agliga , it took me a while to figure out what in the world was going on, which increased the scope of this issue (was supposed to be a size 1). Looking at the entire mechanism, I couldn't find a quick and intuitive solution. Instead of trial and error, which would have taken even more time, I opted for this in the short-term.

I'm good adding the script in, removing the docs, and avoiding the tech debt. Just suggest a change in the PR and I'll merge it.

A quick look through the history shows that it was part of build:site command which was removed. We can add it to the copy command.

skin/package.json

Line 28 in e796673

"build:site": "npm run copy:svgToDocs && npm run build:jekyll",

[agliga]

Not sure how to propose change on a file that was not modified. Its its too much I can just commit it to the 18.3.0 branch directly. No biggie either way.

[ArtBlue]

@agliga , wouldn't that fix require running the build? If so, we should probably keep the docs in and just modify it, right? If I'm running the icon generation, I'd expect the icons to be copied to docs. I don't see any mention in the docs about running build to see the icons in the docs.

[ArtBlue]

@agliga , nm, I see the line where running the build is needed. Ok, I'll make the change.

[agliga]

LGTM! Thanks for this!

[ArtBlue]

@agliga , ok, I made the change to package.json.