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

Update storefront icons #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

beckichoi
Copy link

standardize icons to 32x32 and add title/description

@josephco
Copy link
Contributor

@beckichoi We should remember to go through all icons later and remove any fills/strokes so they can be applied with CSS.

@misscs The updated icons are mostly using stroked paths instead of expanded outlines, so we will have to get used to applying stroke color in the CSS, rather than the fill.

@misscs
Copy link
Contributor

misscs commented Feb 19, 2016

@beckichoi Definitely need to go through and remove references to strokes.

@josephco That's a rather breaking change. We're going to need to update the version to 3 and update the readme to indicate the differences between the two. Additional cleanup PRs against nightshade and nightshade-core will need to happen as well before we can update the version in Nightshade

@misscs
Copy link
Contributor

misscs commented Feb 19, 2016

Great work on this commit @beckichoi! Going to be so nice to have standardized icons in a clean system.

@josephco
Copy link
Contributor

@misscs Yeah, we will have to take care of that. When expanding the stroked paths to use fills, many of the icons became disfigured and lost their proportions. It seems the artwork maintains greater integrity this way (just using paths with stroke), but I will be testing in different browsers today to verify.

@misscs
Copy link
Contributor

misscs commented Feb 22, 2016

  1. Reissue this PR against a new branch v3.0.0-dev
  2. Update Nightshade package.json to point v3 branch
  3. Update CSS in Nightshade-core/nighthade to leverage these new icons

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.

3 participants