Skip to content

DEV: Use themeable site settings for Horizon #33645

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

Merged
merged 8 commits into from
Jul 24, 2025

Conversation

martin-brennan
Copy link
Contributor

@martin-brennan martin-brennan commented Jul 16, 2025

This commit removes the value transformers introduced
to Horizon back in discourse/horizon@274e5f7
and 897d341
in favor of the new themeable site settings introduced in
19af83d , as this is what they
are for.

No migration for existing sites...they will already have
ThemeSiteSetting values from a previous migration to ensure
site setting values were preserved in theme site settings.

We do delete the ThemeField storing the Horizon custom theme
setting definition though, otherwise the UI still shows the old settings.

image

This commit removes the value transformers introduced
to Horizon back in discourse/horizon@274e5f7
and 897d341
in favor of the new themeable site settings introduced in
19af83d , as this is what they
are for.

No migration for existing sites...they will already have
ThemeSiteSetting values from a previous migration to ensure
site setting values were preserved in theme site settings.
Otherwise the old theme settings still show in the UI.

Also fix issue with override indicator for theme site settings,
we should always return a string version of thesetting + default
to the client, it's what it expeccts
@martin-brennan martin-brennan force-pushed the dev/use-themeable-site-settings-horizon branch from 95c6026 to de6075e Compare July 24, 2025 01:48
@martin-brennan martin-brennan merged commit 57fea29 into main Jul 24, 2025
16 checks passed
@martin-brennan martin-brennan deleted the dev/use-themeable-site-settings-horizon branch July 24, 2025 02:20
@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/introducing-horizon-our-newest-theme/369608/41

@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/daily-summary-5am-utc/291851/550

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

Successfully merging this pull request may close these issues.

3 participants