Skip to content

feat: let themes choose their logos and icons on each request - #5322

Merged
imorland merged 2 commits into
2.xfrom
im/theme-image-callables
Oct 29, 2026
Merged

imorland merged 2 commits into
2.xfrom
im/theme-image-callables

Conversation

@imorland

@imorland imorland commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Fixes: no issue filed.

Changes proposed in this pull request:

#5319 lets a theme set the forum's logos and icons, but only as fixed files. A theme with switchable brand sets, seasonal ones for example, still has to write the image settings itself, where the admin can overwrite them. And while a theme sets an image, code reading the image settings directly still gets the admin's old upload.

  • Callables: every image argument of Extend\Theme (logo(), logoDarkMode(), emailLogo(), favicon() and its SVG, appIcon() and its background) also accepts a callable that returns one, with its parameters resolved from the container. Returning null leaves the image to the admin. Returning a different file draws the image again.
  • The image settings: while a theme sets an image, logo_path, logo_dark_mode_path, favicon_path, email_logo_path and email_logo_size hold the drawn file, when it is the kind of file an upload stores there. An SVG logo leaves logo_path as it was. The admin's values come back when the theme stops setting the image. fof/seo, fof/pwa and ianm/twofactor read these settings.
  • Replaced files are kept for 30 days, for cached pages, the cached manifest and /favicon.ico redirect, and emails already sent, and taken back if the image returns to them. They were deleted at once.
  • Signatures use file contents, not the absolute path and modification time, so servers deployed separately agree on what was drawn instead of drawing over each other.

Docs: flarum/docs#694

Reviewers should focus on:

  • Callables run each time ThemeImages is built, about twice per forum page, so they should be cheap and give the same answer within a request.
  • has() now brings the stored state up to date first. Nothing did once a theme stopped setting every image, so its files were never removed.
  • A file's hash is kept in the cache store, keyed by its path, size and modification time, so the file is only read again when one of those changes.
  • The theme_images state format changed. It is unreleased, so a 2.x snapshot install just draws its theme images once more.
  • ThemeImagesTest allows repeated settings writes: drawing writes each image's settings once.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Frontend changes: tests are green (run yarn test in js/).
  • Frontend changes: tests have been added, or are not appropriate here.
  • Backend changes: tests are green (run composer test).
  • Backend changes: tests have been added, or are not appropriate here.
  • Where applicable, changes are suitable for all supported database drivers (MySQL, MariaDB, PostgreSQL, SQLite). (No database changes.)
  • Core developer confirmed locally this works as intended.

@imorland imorland added this to the 2.1 milestone Oct 29, 2026
@imorland
imorland marked this pull request as ready for review October 29, 2026 14:31
@imorland
imorland requested a review from a team as a code owner October 29, 2026 14:31
@imorland
imorland merged commit 36ee2a8 into 2.x Oct 29, 2026
35 of 36 checks passed
@imorland
imorland deleted the im/theme-image-callables branch October 29, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants