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

EuiIcon with custom SVG #1702

Closed
timroes opened this issue Mar 8, 2019 · 3 comments
Closed

EuiIcon with custom SVG #1702

timroes opened this issue Mar 8, 2019 · 3 comments

Comments

@timroes
Copy link
Contributor

timroes commented Mar 8, 2019

In Kibana we have a couple of places where a plugin can register something that has an icon (e.g. a new visualization type, a new management section that will show up in the menu, etc.). Usually we allow those users to either register an EuiIcon type (as a string) or a custom SVG, currently as a data URL, but since we're now using React in most places, we could instead accept an SVG as JSX.

Right now all the places that are consuming this, we make some kind of differentiation and either put an <EuiIcon /> or a <img /> (e.g. see the vis type icon in the 'create new vis' dialog).

Since we'll use that in quiet some places and we already see the first users complaining in places we (accidentally) removed that feature, that same behavior will become more and more common. Right now we're needing to make this differentiation in a couple of places, why it would make sense to have a shared component for that. Since this is completely independent from specific plugins we would need to place this at some root level UI components in Kibana (which currently doesn't exist), why I wanted to start discussing if it's maybe worth allowing EuiIcon to alternatively take in a JSX of an SVG instead of the type, and render that instead.

Another issue when we put that outside of EuiIcon is the following: Currently some EUI components (e.g. ListGroup) only allow us to set an EuiIcon type, which--even if we put that custom component into Kibana--wouldn't allow us to use it in those components (at the moment for example a problem, in the dashboard add panel, which uses EuiListGroup and custom visualization plugins can't show their icons in that list, in contrast to all other visualizations). So I think if we decide not to allow custom SVG in EuiIcon we should make sure that all the places currently only accepting an EuiIcon type, should alternatively also accept an JSX element as an icon (e.g. as the side nav component does). Even if we put it into EuiIcon direction, we would need to change those places to also allow the JSX SVG instead of the plain string type.

Btw, I mentioned putting it into EuiIcon directly here all the time, but of course it could also be an option having it as a separate component in EUI.

What are your thoughts about that?

@snide
Copy link
Contributor

snide commented Mar 8, 2019

This was listed on our short term roadmap already and is planned. #1398

The plan is to support any SVG and to break apart the component so it doesn't have the annoying 500kb compile warning.

I'll leave this issue up since it best explains the need.

@cchaos
Copy link
Contributor

cchaos commented Mar 18, 2019

The best way to provide this currently is to use an EuiImage with the custom svg source url as the src if the passed icon is not of EuiIcon.type. Implementations should know whether the source of the icon will be an EuiIcon.type or an image path and use the correct component accordingly.

@snide
Copy link
Contributor

snide commented May 7, 2019

Closed via #1924

@snide snide closed this as completed May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants