Get rid of <sl-icon> #122
Get rid of <sl-icon> #122
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
oyamauchi
force-pushed
the
owen.sl-icon
branch
from
f087001
to
ee08253
Compare
## Description The SVG project icons can't be included via `<img>` because they are colored using `currentColor`. They also can't be included via Parcel's `bundle-text` named pipeline becase The way we used `<sl-icon>`, it was fetching the svg file over the network and dynamically inserting it into the DOM. This instead uses another Parcel plugin to import an svg file as a React component at build time. Getting it to work is kinda finicky. Ssee parcel-bundler/parcel#7587, plus the workaround I ended up using in the not-merged PR parcel-bundler/parcel#7711. The list of transformers for `jsx:*.{js,jsx}` is copied from Parcel's default config for those file types. The icons are set to width `1em` to allow them to be sized by the font-size at the usage site, like `<sl-icon>`. ## Test Plan Look at the project icons in all contexts where they appear: project multiselect, icon tab bar in pill mode, icon tab bar in dropdown mode.
oyamauchi
force-pushed
the
owen.sl-icon
branch
from
ee08253
to
50e9773
Compare
| @@ -9,7 +8,7 @@ import { Spinner } from './spinner'; | |||
| export type Option<T extends string> = { | |||
| value: T; | |||
| label: string; | |||
| iconURL?: URL; | |||
There was a problem hiding this comment.
Would this be more idiomatic React as just Icon?: React.ReactElement? And then <Icon/> instead of {icon()}?
Definitely don't feel strongly about it, but I had to scan back and forth a couple of times to work out how getIcon was plumbed through into renderTag.
There was a problem hiding this comment.
This comment was supposed to be on the added line not the removed line. Makes sense in side by side diff but confusing in the main PR view!
There was a problem hiding this comment.
Is it right that this diff replaces the locales import helpers?
There was a problem hiding this comment.
Yeah, those have actually been unused for a while and I forgot to remove them.
Description
The SVG project icons can't be included via
<img>because theyare colored using
currentColor. They also can't be included viaParcel's
bundle-textnamed pipeline becaseThe way we used
<sl-icon>, it was fetching the svg file over thenetwork and dynamically inserting it into the DOM. This instead uses
another Parcel plugin to import an svg file as a React component at
build time. Getting it to work is kinda finicky. Ssee
parcel-bundler/parcel#7587, plus the workaround I ended up using in
the not-merged PR parcel-bundler/parcel#7711. The list of transformers
for
jsx:*.{js,jsx}is copied from Parcel's default config for thosefile types.
The icons are set to width
1emto allow them to be sized by thefont-size at the usage site, like
<sl-icon>.Test Plan
Look at the project icons in all contexts where they appear: project
multiselect, icon tab bar in pill mode, icon tab bar in dropdown mode.