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

Persist font settings when cloning a theme #678

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

mikachan
Copy link
Member

@mikachan mikachan commented Jul 1, 2024

This PR should apply the "Save Fonts" save option when cloning a theme via Create Theme -> Clone Theme. This means that any font settings and files are copied over to the new cloned theme, including updating the file asset paths.

Fixes #677.

To test:

  1. Install and activate some custom fonts using the Font Library.
  2. Go to the CBT menu, Create Theme -> Clone Theme.
  3. Name your theme.
  4. Check the theme.json of the newly cloned theme: ensure that the fontFace.src paths are relative to the cloned theme. i.e. the paths start with file:. and not an absolute URL.
@mikachan mikachan added the bug Something isn't working label Jul 1, 2024
Copy link
Contributor

@matiasbenedetto matiasbenedetto left a comment

Choose a reason for hiding this comment

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

I think this could work but we would be removing the possibility of having linked fonts in a cloned theme. All the fonts will be downloaded and made local as we don't have that possibility in WordPress core yet I think it's OK and nobody will expect the opposite. I will try in action to see if it solves the problem.

Copy link
Contributor

@matiasbenedetto matiasbenedetto left a comment

Choose a reason for hiding this comment

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

This works well fixing #677
Let's merge it.

@mikachan
Copy link
Member Author

mikachan commented Jul 2, 2024

Thanks @matiasbenedetto!

I think this could work but we would be removing the possibility of having linked fonts in a cloned theme.

Ah yes, we can address this later once Core supports it, like you say 👍

@mikachan mikachan merged commit 070c444 into trunk Jul 2, 2024
2 checks passed
@mikachan mikachan deleted the fix/clone-font-settings branch July 2, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
2 participants