feat(Icon): Partial new icon replacements and updates #1809
Conversation
dlnr
changed the title
…into feat/DES-943-new-icons
…into feat/DES-943-new-icons
| 'PhoneFillIcon', | ||
| 'SpeechBalloonFillIcon', | ||
| 'WarningFillIcon', | ||
| ] |
There was a problem hiding this comment.
Not sure about the naming. Calling these icons *Filled* and the set filledIcons would be more correct.
There was a problem hiding this comment.
@dlnr, can we change the naming convention for the ADS without consulting the design team? Can we later backport the naming convention to Figma if we agree to do that?
There was a problem hiding this comment.
Yes, that will probably happen. I've suggested to the design that we'll comment the filenames we're changing.
| 'ListIcon', | ||
| 'MagnifyingGlassIcon', | ||
| 'MapIcon', | ||
| 'MastadonIcon', |
There was a problem hiding this comment.
| 'MastadonIcon', | |
| 'MastodonIcon', |
There was a problem hiding this comment.
Probably out of the scope of this story, but I think the Municipality of Amsterdam should consider to support sharing with Bluesky as well. Bluesky is quickly becoming the leading alternative to X/Twitter and is more popular than Mastodon.
Bluesky lives in the same "Fediverse" ecosystem as Mastodon and is interoperable to some degree.
There was a problem hiding this comment.
@dlnr, we can apply this name change in the ADS regardless of the naming in Figma, right?
There was a problem hiding this comment.
I've renamed it in Figma and will run the export as soon as all the shapes are named properly.
| 'ExternalLinkIcon', | ||
| 'EyeIcon', | ||
| 'EyeOffIcon', | ||
| 'FacebookIcon', |
There was a problem hiding this comment.
Would it be helpful for discoverability and search purposes to prefix icons like "Facebook", "Instagram", "LinkedIn", "Mastodon", "XTwitter", etcetera with Social* or Share*? This way they are grouped together as they will often be used together.
There was a problem hiding this comment.
@dlnr, this seems more like a choice for us as ADS maintainers. Not something we have to bother design with.
There was a problem hiding this comment.
Also something to suggest in Figma first, I would like to prevent renaming files manually after the export.
There was a problem hiding this comment.
If you agree with this suggestion, can you take it up with the designers via Figma?
There was a problem hiding this comment.
The fill icon doesn't really seem to work. I think the coherence is lost and it doesn't really look like an old-fashioned "folder". On a smaller scale it doesn't look much better.
There was a problem hiding this comment.
@dlnr, I'm currently looking at the de# Figma and I have the impression that the SVG is not correct in Storybook. Take a look: https://designsystem.amsterdam/demo-DES-943-new-icons/?path=/docs/brand-assets-icons--docs
|
@RubenSibon feedback on the design or naming of the icon is more useful in the the Figma file: https://www.figma.com/design/9IGm6IdPUYizBNGsUnueBd/Amsterdam-Design-System?node-id=6852-5124 make sure you "@" mention Sieger and/or Anna Kay |
VincentSmedinga
changed the title
Describe the pull request
Thank you for contributing to the project!
Please use this template to help us handle your PR smoothly.
What
The new iconset adds and replaces icons in the current set.
Why
We are ready to release the current set of new icons.
How
The page Brand > Assets > Icons now displays icons in the following sections:
Checklist
Before submitting your pull request, please ensure you have done the following. Check each checkmark if you have done so or if it wasn't necessary:
Additional notes