-
Notifications
You must be signed in to change notification settings - Fork 4
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
New Icon Mixin #1262
New Icon Mixin #1262
Conversation
🦋 Changeset detectedLatest commit: 3008904 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for ilo-ui-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ilo-ui-twig-develop ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ilo-ui-twig ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
356e109
to
3008904
Compare
Description
This PR introduces a new mixin that supports base64 icon placement with new theme support (#999)
API
The API for the new mixin is almost the same, to maintain the same DX
@ilo-org/icon
,required
Implementation
Base64 encoded
svg
has no direct support for CSS variables since it is evaluated as a static string, one possible solution to get rid ofdataurilicon
was to eliminate the base64 encoded svgs but it has its disadvantagesreact
and fortwig
and having a single source of truth from the styles package is a good thingicon
component for bothtwig
andreact,
but fortwig
usersjs
bundle for the icon component will become the required dependency, which isn't common behavior for thedrupal
&symphony
projectstwig
andreact
have radically different coverage for the components in terms ofjs
so maintaining icons from the styles package will increase the phase for the developmentSo, the current implementation keeps the icons in
base64
and adds CSS variables support with the utilization of themask-image
. You can read more about mask-image here;tldr, we place the icon as a mask element and add color to it using background-color.There is a disadvantage too, if one
node
uses bothicon
mixin and background color they should be split markup-wise or pseudo selectors should be usedComponent migration
First I tried to port all the components, but it would
block
further development of the components, so we can address every component in the scope of #971. The PR has a few examples of how to port the component to the new mixin.Migrated components
dataurlicon