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

Inline the avatar mark to remove the react-dom/server dependency #193

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

sandhose
Copy link
Member

This unnecessarily adds a react renderer to the bundle/dependencies just for a simple SVG that doesn't change.

It's also a step towards removing SVGR from this project.

@sandhose sandhose requested a review from a team as a code owner June 20, 2024 15:13
@sandhose sandhose requested review from dbkr and robintown and removed request for a team June 20, 2024 15:13
Copy link

cloudflare-workers-and-pages bot commented Jun 20, 2024

Deploying compound-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 66b8fea
Status: ✅  Deploy successful!
Preview URL: https://407c8746.compound-web.pages.dev
Branch Preview URL: https://quenting-inline-avatar-mask.compound-web.pages.dev

View logs

@sandhose sandhose force-pushed the quenting/inline-avatar-mask branch from fd5fd47 to 643929a Compare June 20, 2024 15:14
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a step towards making it harder to simply diff files between Figma and git, or better yet automate them for design to own them

@sandhose sandhose force-pushed the quenting/inline-avatar-mask branch from 643929a to 828459a Compare June 20, 2024 15:14
@sandhose
Copy link
Member Author

This is a step towards making it harder to simply diff files between Figma and git, or better yet automate them for design to own them

I would agree for icons in the design tokens, but it feels like this path was manually crafted?

@t3chguy
Copy link
Member

t3chguy commented Jun 20, 2024

React-SDK had a similar mask (before switching to Compound Avatars) which came from Figma

@sandhose
Copy link
Member Author

Fine, I replaced that with a raw import of the SVG. I guess the bundle size would be slightly bigger as the SVG has comments in it, but it's nothing compared to react-dom/server

Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sane

@sandhose sandhose merged commit 3b82efe into main Jun 20, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants