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

Plans Next: Migrate assets/svgs imported in features-list to packages #88182

Closed
Tracked by #79004
chriskmnds opened this issue Mar 5, 2024 · 2 comments · Fixed by #88186
Closed
Tracked by #79004

Plans Next: Migrate assets/svgs imported in features-list to packages #88182

chriskmnds opened this issue Mar 5, 2024 · 2 comments · Fixed by #88186

Comments

@chriskmnds
Copy link
Contributor

chriskmnds commented Mar 5, 2024

What

Part of addressing #79004

Migrate the following icons or images into a publicly accessible package:

Why

These are being imported into lib/plans/features-list which is being migrated to @automattic/calypso-products for importing and using outside of the Calypso environment.

How

One idea would be to create a @automattic/calypso-assets package (or something of this nature) and initiate the package with migrating SupportIcon and Theme2Image there. Moving along, we could be migrating more things until the entirety of /client/assets migrates.

Other options:

  • SupportIcon: This could fit into @automattic/components/icons
  • Theme2Image: This is a jpg that's only used in features-list. It could fit into @automattic/calypso-products/assets (new folder)
  • MaterialIcon: This could fit straight into @automattic/components (similar to the WP logo there)
@chriskmnds chriskmnds self-assigned this Mar 8, 2024
@chriskmnds chriskmnds changed the title Plans Next: Migrate assets/svgs imported in features-list to @automattic/components Plans Next: Migrate assets/svgs imported in features-list to packages Mar 8, 2024
@tyxla
Copy link
Member

tyxla commented Mar 8, 2024

If you really insist, you can create a new assets package, but for a couple of assets, it feels like overkill. What you suggested in "other options" seems OK to me and that's what I'd likely prefer to do.

@chriskmnds
Copy link
Contributor Author

If you really insist, you can create a new assets package, but for a couple of assets, it feels like overkill. What you suggested in "other options" seems OK to me and that's what I'd likely prefer to do.

Thanks @tyxla. It makes sense. "other options" were also my first/immediate take on this, so will go with that and avoid a new package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants