search

TryGhost / Ghost

Independent technology for modern publishing, memberships, subscriptions and newsletters.
https://ghost.org
MIT License
47.62k stars 10.38k forks source link

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

Open cathysarisky opened 1 week ago

cathysarisky commented 1 week ago

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

cathysarisky commented 1 week ago

Solo and Episode are impacted.

minimaluminium commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

@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.)