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.

Sign up for GitHub

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

Jump to bottom

Move static assets to web-common #1814

Merged
merged 3 commits into from
Mar 8, 2023
Merged

Move static assets to web-common #1814

merged 3 commits into from
Mar 8, 2023

Conversation

ericpgreen2
Copy link
Contributor

@ericpgreen2 ericpgreen2 commented Feb 23, 2023 •
edited
Loading

For web-admin to leverage our static assets (currently, our fonts & favicon), I've moved them from web-local/static into web-common/static and configured our SvelteKit projects, web-local and web-admin, to read the web-common/static directory.

I looked into keeping robots.txt in a separate web-admin/static folder, but SvelteKit does not yet support multiple static directories.

Part of #1780.

begelundmuller
begelundmuller reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Can it still also have local and admin specific assets as well? It would be nice to be able to decouple those.

Also, just another question, is there an NPM package for Inter that we could use so we don't need to copy them into the repo?

begelundmuller
begelundmuller approved these changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Approving since my questions above can be resolved later if necessary. This looks good

@ericpgreen2
Copy link
Contributor Author

Moving my Slack question to @hamilton here –

Thoughts on Benjamin's fonts idea? I found this Inter NPM package, but haven't found anything yet for MD IO.

@ericpgreen2
Copy link
Contributor Author

Thoughts on Benjamin's fonts idea? I found this Inter NPM package, but haven't found anything yet for MD IO.

@hamilton mentioned that we're using the "tabular numbers" OpenType feature. I found that fontsource has quite limited OpenType support and does not offer tabular numbers.

I think I'll leave the Inter files in the repo and we can continue this research another time. Another idea is to use the Inter distribution on Cloudflare's CDN.

@ericpgreen2
Copy link
Contributor Author

Can it still also have local and admin specific assets as well? It would be nice to be able to decouple those.

Maybe there's a creative workaround, but otherwise not until this SvelteKit issue is solved.

Given we still have the fonts in the repository (see comment above), we can either:

  1. decouple local and admin assets & duplicate fonts
  2. couple local and admin assets & not duplicate fonts

I feel option 2 (current approach) is best until we get the fonts out of the repo.

@begelundmuller thoughts or merge?

@begelundmuller
Copy link
Contributor

Makes sense – let's do the current approach until the fonts are removed. Merge!

@ericpgreen2 ericpgreen2 merged commit 1f77017 into main Mar 8, 2023
@ericpgreen2 ericpgreen2 deleted the move-static-assets-to-web-common branch March 8, 2023 12:53
djbarnwal pushed a commit that referenced this pull request Aug 3, 2023
@ericpgreen2
Move static assets to web-common (#1814)
13140ec 
* Move static assets to `web-common`

* Delete `web-local/static`

* Fix prettier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@begelundmuller begelundmuller begelundmuller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericpgreen2 @begelundmuller