search

Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 799 forks source link

Import material icons from library #11901

Closed simison closed 3 weeks ago

simison commented 5 years ago

Since we decided using Material icons on our Gutenberg blocks (https://github.com/Automattic/wp-calypso/issues/28519, https://github.com/Automattic/wp-calypso/issues/28500), we'll have plenty more of these coming up.

Would be great to be able to import these directly from a library instead of manually copying them over like we're doing now.

We'll need the actual SVG contents, not just image path and image copied with bundle like we do for the rest of the images:

https://github.com/Automattic/wp-calypso/blob/70d34aeda7ed952d929269bf60b9cf60e6dad5db/client/gutenberg/extensions/simple-payments/editor.js#L29-L34

Some candidate source libraries:

(via https://github.com/Automattic/wp-calypso/pull/28501#issuecomment-438417992)

blowery commented 5 years ago

Ah, looks like Gutenberg has just been inlining them as needed.

It looks like Gutenberg has a custom SVG wrapper for a11y concerns. Rather than pulling into one of those third party components, which has it's own set of opinions, would it make sense to add a new icons package to Gutenberg and expose the icons that way?

@material-ui/icons returns a ReactElement with their custom wrapper material-design-icons hasn't been updated in ... 2 years? /cc @lynnmercier

simison commented 5 years ago

It looks like Gutenberg has a custom SVG wrapper for a11y concerns.

Yep! Gutenberg SVG primitives add these three attributes for better accessibility:

role: 'img',
'aria-hidden': 'true',
focusable: 'false',

We should try to PR these to upstream to a library we choose to use so that it benefits everyone, not just us.

Creating our own might indeed make us faster and in future allow us putting WordPress specific icons there, too.

simison commented 5 years ago

Noting that Gutenberg's SVG wrapper is not only for a11y concerns but also to make SVGs compatible with React Native:

https://github.com/WordPress/gutenberg/blob/ec1a4cbe126d860f0feb0dda8c0c8f8c1d38419b/packages/components/src/primitives/svg/index.native.js

simison commented 5 years ago

This came up at the Core, too: https://github.com/WordPress/gutenberg/issues/9647

simison commented 5 years ago

Convo is up again https://make.wordpress.org/design/2019/02/06/meeting-notes-for-february-6-2018/

simison commented 5 years ago

Relevant reading https://make.wordpress.org/design/2019/02/12/whats-next-for-dashicons/

simison commented 5 years ago

It looks like Gutenberg has a custom SVG wrapper

Might change in future: https://github.com/WordPress/gutenberg/issues/14628

blowery commented 5 years ago

Relevant work over in Calypso: https://github.com/Automattic/wp-calypso/pull/32172

simison commented 5 years ago

We now have internal NPM package where we can pull from https://github.com/Automattic/wp-calypso/tree/3e69a83a497e8e2c0708d074311398cd98037df0/packages/material-design-icons

simison commented 3 weeks ago

We now also have @wordpress/icons library.