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

New typography feature prevents theme's custom.typography value from appearing #21644

Open
1 task done
cathysarisky opened this issue Nov 18, 2024 · 4 comments
Open
1 task done
Labels
needs:triage [triage] this needs to be triaged by the Ghost team

Comments

@cathysarisky
Copy link
Contributor

Issue Summary

I have a (now-outdated) theme with a custom.typography value. It used to show up in the admin dashboard for theme customization. Now it doesn't. Renaming the variable to typographyb causes it to appear, so I suspect a conflict with the new typography dropdowns.

Steps to Reproduce

Install an older theme with a custom.typography setting defined in package.json.
Observe that the custom value does not appear with the other custom values in the admin dashboard.

Ghost Version

5.101.1

Node.js Version

18

How did you install Ghost?

local install.

Database type

SQLite3

Browser & OS version

Chrome, windows 10, recent version

Relevant log / error output

No response

Code of Conduct

  • I agree to be friendly and polite to people in this repository
@github-actions github-actions bot added the needs:triage [triage] this needs to be triaged by the Ghost team label Nov 18, 2024
@cathysarisky
Copy link
Contributor Author

Solo and Episode are impacted.

@minimaluminium
Copy link
Member

Hey @cathysarisky. Just to confirm, it only happened when you used one of the official themes, is that correct? Seeing that you mentioned Solo and Episode were impacted, I assume that was the case, but just wanted to be extra sure.

@cathysarisky
Copy link
Contributor Author

cathysarisky commented Nov 20, 2024

Hey @minimaluminium , I saw it first on a client's site, which is a heavily-modified version of episode. It was installed by uploading a zip, not via the one-click deploy.

I don't know whether any non-official themes use 'typography' as a custom variable, but it wouldn't surprise me if some do.

@cathysarisky
Copy link
Contributor Author

@minimaluminium , I saw a comment from you that has disappeared. So I think I understand that this is the intended behavior for official themes with a typography attribute, and you're checking for theme name and maybe now author. (Although I don't normally update those fields, it isn't hard to do so if it's documented.) But... rather than trying to come up with some logic that removes the existing displayed typography settings from two variables, why not just update those two themes? That way, anyone running an old version still has access to episode's "wide" and "narrow" typography settings. I'm not sure I see the benefit of hiding the setting unless you're also patching out the use of the custom variable. (Which, when I checked episode this morning, you hadn't.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:triage [triage] this needs to be triaged by the Ghost team
Projects
None yet
Development

No branches or pull requests

2 participants