[3.0] Theme

[live627]

This is the new theme started by @TwitchisMental

I still need to convert several form grids to use CSS grids instead of floats. Also the grids abuse definition list markup, something tat has always annoyed me from the very beginning.

Submitting as a draft now to get early feedback.

[jdarwood007]

If we do this for the avatar image, shouldn't we use a alt text for the name?

[DiegoAndresCortes]

Should use href or url then.
Or should 'image' provide it by default? It would be good in case an external avatar is missing in other parts of the forum.

[jdarwood007]

I think either is valid. I believe it just needs something to be valid for screen readers. Since its a link to the users profile and we removed the name, the alt should have it.

[DiegoAndresCortes]

Then I think 'image' should provide the alt text by default. An external avatar could suddenly not load or disappear, would be good to have it.

[live627]

I notice that a few forms would disable textareas on submit, such as the posting form. I've changed this to do all forms by using the new HTML property to make the entire form inert.

[dragomano]

Icons in the admin area are not indented enough:

Unable to load the '' template at Forum Maintenance - Routine:

SCEditor doesn't have a dark theme?

Instead of "X Posts" and "Y Topics", maybe it would still be better to keep "Posts: X" and "Topics: Y" as it is now?

[DiegoAndresCortes]

Icons in the admin area are not indented enough:

I’m working on this

SCEditor doesn't have a dark theme?

Can be fixed later too, but also requires some tweaks.
Check this topic: https://www.simplemachines.org/community/index.php?topic=586626.0

Instead of "X Posts" and "Y Topics", maybe it would still be better to keep "Posts: X" and "Topics: Y" as it is now?

I don’t have a preference, we might discuss it later for sure.

[jdarwood007]

@live627 Can you rebase this or merge and fix conflicts?

[DiegoAndresCortes]

We have this pending
live627#44

Will start sending more in upcoming weeks, but it could be merged, rest of changes are more focused so can be in separated PR's

[live627]

Once I can get around to testing and merging that, I'll rebase this branch again

[dragomano]

checkeiv => checked?

[dragomano]

<label>...</div> => <label>...</label>?

[live627]

@jdarwood007 I moved some of the login javascript to its own file but don't know how to test cors requests.

[jdarwood007]

When this is closer to ready I can check it. You need to run 2 domains (or 2 subdomains) and have the forum on one and the script on another. A simple SSI page where you can open the user area popup is good enough for most things. Sending credentials such as on the login form may also need to be tested, but when you can open the user area, the CORS request works.

[albertlast]

@Sesquipedalian @sbulen @jdarwood007 @live627 — I have taken a run at the split, and I think it is tractable. First PR is up: #9332.

Why the earlier attempts stalled

@sbulen is right that the history cannot be split. 151 commits, most of them merges, changes layered on changes. Trying to separate commits is the salt-and-pepper problem.

But the history is not what needs splitting. The end state is. And that turns out to be easy, because this branch is currently in good shape: @jdarwood007 has kept it merged with release-3.0, so as of today it is behind_by=0 and mergeable. That makes its tree usable as a frozen reference.

So the procedure is:

1. Fetch the PR head once, as theme-ref. Never commit to it except to merge release-3.0 forward.
2. For each part: branch from release-3.0, then take that part's slice out of theme-ref — whole files where a file belongs to one part, hunk by hunk where it does not. index.css still has its /* section */ comments in both versions, and they make good seams.
3. Each part has to stand on its own on top of release-3.0. A template part carries its own CSS; a CSS part must not assume markup that has not landed yet.
4. After a part merges, merge release-3.0 forward into theme-ref and that slice drops out of the remaining delta by itself.

No cherry-picking, no rebasing, and the conflict burden goes down every round instead of up. When git diff release-3.0 theme-ref is empty, this PR gets closed.

I would suggest keeping this PR open as the tracking issue for the series, marked as not-for-merge, so the attribution to @live627, @TwitchisMental, @DiegoAndresCortes, @sbulen and @jdarwood007 stays visible.

Three things that are no longer true

Worth flagging, because they change what actually needs doing:

• The dark mode plumbing has already landed in release-3.0. Theme.php resolves theme_colormode, builds ['light','system','dark'] and already calls loadCSSFile('dark.css', …) with the prefers-color-scheme media attribute. Themes.template.php and ThemeOptions.php already expose the settings, and index.template.php already emits data-mode. The theme simply never ships a dark.css, so has_dark_mode is false. So dark mode is a CSS file plus one flag now — not the infrastructure work it looked like in December.
• Variants are a regression in this PR, not a missing feature. It sets theme_variants = []. But variables.css is built on --primary-color-hue, which means a variant becomes about five lines. That should answer @jdarwood007's concern.
• variables.css gates almost everything else. Today's index.css has zero var(--…); the version here has 1268 of them.

The proposed breakdown

Grouped into waves, and numbered within each wave — [3.0] Theme split (wave 1, part 2) — …. I have deliberately not put a running total on any of this. I do not know yet how many parts it takes, and a made-up denominator would only be wrong later.

A wave is a set of parts that can be reviewed in parallel. The next wave starts from wherever the previous one ended up, so nothing depends on guessing the shape of the whole series in advance.

Wave 1 — no theme template changes at all. Reviewable now, in any order:

1. [code] BBCode emits <pre><code>, buttons built in JS — [3.0] Theme split (wave 1, part 1) — Emit code blocks as <pre><code> and build their buttons in JS #9332
2. Forum maintenance tasks as one pick-one form — [3.0] Theme split (wave 1, part 2) — List the maintenance tasks as one pick-one form #9333. Also fixes @dragomano's "Unable to load the '' template" report from this thread
3. Drop the unused jQuery Custom Scrollbar — [3.0] Theme split (wave 1, part 3) — Drop the unused jQuery Custom Scrollbar #9334
4. Replace the jQuery At.js and Caret.js plugins — [3.0] Theme split (wave 1, part 4) — Replace the jQuery At.js and Caret.js plugins #9335

Two more belong to this group but are not ready: the editor/SCEditor work needs subdividing, and the drafts work has problems of its own. Details in my follow-up comment.

Wave 2 — the CSS foundation. Sequential, and the first one gates the rest:

1. Design tokens: add variables.css, convert hard-coded values to var(--…). Acceptance test is "nothing changes visually"
2. Dark mode: add dark.css, flip has_dark_mode
3. Variants via --primary-color-hue
4. RTL → CSS logical properties (deletes rtl.css and calendar.rtl.css)
5. icons.css

Wave 3 — the wide-ranging one. index.template.php, header/menu/footer, GenericMenu.template.php, responsive.

Wave 4 — one part per area, independent of each other once wave 3 lands: calendar, admin centre, board index, message index, topic display and postbit, posting, profile, personal messages, login/register/reminder, and the long tail (Search, Stats, Memberlist, Recent, Help, Errors, Moderation, …).

About #9332

It supersedes #8891, which was @live627's own attempt at exactly this part back in August. That one has gone stale — it still refers to scripts/jquery.sceditor.smf.js, which has since been renamed.

Splitting also surfaces things that were invisible inside the big diff. Three examples from this one part alone:

• The theme branch leaves Errors.template.php on the old code-block markup, so the error log's Select and Expand buttons would have silently stopped working.
• ssi_examples.php (33 blocks), UpgradeTemplate.php and ExportProfileData.php all consume that markup too, and none of them are in this PR's file list at all.
• [code=php] on a snippet without an opening <?php loses its syntax highlighting, which has nothing to do with the markup change. I kept the old behaviour.

That is the argument for the split in a nutshell — those were not findable at 110 files.

I will keep working through wave 1. Happy to change the grouping if you would rather have it carved up differently.

[albertlast]

@Sesquipedalian — status update on the split, and a few questions on how you want to proceed.

Open so far

Wave 1 is up, titled so they are easy to track. Numbering is per wave, with no running total — I do not know how many parts this takes, and a guessed denominator would only be wrong later.

• [3.0] Theme split (wave 1, part 1) — Emit code blocks as <pre><code> and build their buttons in JS #9332 — wave 1, part 1 — [code] BBCode emits <pre><code>, buttons built in JS
• [3.0] Theme split (wave 1, part 2) — List the maintenance tasks as one pick-one form #9333 — wave 1, part 2 — list the maintenance tasks as one pick-one form
• [3.0] Theme split (wave 1, part 3) — Drop the unused jQuery Custom Scrollbar #9334 — wave 1, part 3 — drop the unused jQuery Custom Scrollbar
• [3.0] Theme split (wave 1, part 4) — Replace the jQuery At.js and Caret.js plugins #9335 — wave 1, part 4 — replace the jQuery At.js and Caret.js plugins

All four are mergeable and were tested against a running forum, not just linted — and not only one at a time. I keep a branch that is release-3.0 with all four merged together, and re-run every part's checks on it, because a part passing on its own branch does not prove the parts still compose. That caught #9333 shipping without its main fix, which CI was perfectly happy with because the failure was at runtime.

Only #9333 touches a theme template, and it is the template the change is about.

Two parts I dropped rather than ported

Self-hosting FontAwesome. This branch adds eight webfont files, but its own fontawesome.min.css contains no @font-face rules, so nothing references them. Porting it would add about 1.1 MB of orphaned binaries.

It did turn up a separate bug though: on release-3.0, setting the FontAwesome source to "local" serves the icon CSS with no font behind it, so every icon breaks. That looks worth its own issue, but it is new work rather than a slice of this PR, so I left it alone.

Moving JumpTo into script.js. release-3.0 already moved JumpTo and IconList the other way, into topic.js, in d591cd6, and has since added Ctrl+Enter support and a redirect fix on top. Taking this branch's version would be a revert.

Two parts I stopped on

The editor and drafts work is a different size from the rest, and it needs subdividing before it is reviewable.

Drafts is separable in principle — Editor on release-3.0 already merges $editorOptions['options'], so the plugin can be wired without the wider editor refactor. But the pmDrafts plugin here does not work as written:

• getRecipient() and getFormData() use form and editor, which are declared inside this.init rather than in the plugin closure, so both throw ReferenceError when called.
• It posts id_pm_draft; the server reads $_POST['id_draft'], so each autosave would create a new draft instead of updating the existing one.
• It posts recipient_to[] repeated, which makes $_POST['recipient_to'] an array, but Draft.php does explode(',', $_POST['recipient_to']) — a TypeError, fatal on every PM autosave.

There is also a leftover console.log(XMLDoc) in the shared drafts plugin.

Fixing those and checking them against the server contract is real work rather than porting, so I would rather not push it as part of a "split up the existing PR" series without asking first.

The editor part splits further than I had it. A lot of the 619-line reduction in sceditor.plugins.smf.js is code being moved into the new plugin files, so it naturally becomes: quote-fast, xml-preview, and then whatever modernisation is left.

A pattern worth mentioning

Every part so far has turned up something that was invisible at 110 files. The error log's Select and Expand buttons would have silently stopped working. template_choose_boards() emits brd[] while TopicRemove::old() reads $_POST['boards'], so pruning by selected boards would not do what the form says. The ported At.js render() loops to the configured limit instead of the result count, so it threw before ever showing the dropdown — mentions looked completely dead for any search narrow enough to be useful.

None of that was findable in the combined diff. It is the best argument I have for doing this the slow way.

Questions

1. Does the grouping work for you, or would you rather have it carved differently before I go further?
2. For the editor and drafts: subdivide as above and carry the fixes inside those PRs, or land the port as-is and raise the defects separately?
3. Should fixes like these keep going into the split PRs, or would you prefer them as standalone bug PRs against release-3.0 so the split stays a pure move?
4. Happy for me to start wave 2 (the CSS foundation), or would you rather review what is open first? Design tokens gate most of what follows, so that one is on the critical path.

[jdarwood007]

@albertlast
While I've done what i can to keep it in a mergeable state, there is no guarantee that things function without errors. Its been a lot of work to get it into a mergeable state. So we still need to take precautions with it.

However, as we merge up the theme changes, we can merge them into this one and try to see what is left as we get closer.

[albertlast]

Well i don't think you need to invest any more time into it,
since sesqui asked for are more splitted version approache of this pr (so work doesn*t get lost),
which i deliver him, in this way.
also in best case i alway open only pr for actual wave, when the wave is done the next wave get created by the llm.

[jdarwood007]

If thats what we thihk we should do. I did just send up a few typos from the last merge that I didn't catch.

[albertlast]

When you look at the pr,
you notice that the llm also is test the coding and fixes small issue which mostly exists in this original version here.
like the #9335 "one fix on top"

[albertlast]

@Sesquipedalian — wave 1 is finished. Six PRs, all mergeable, all checks green.

#9332	wave 1, part 1 — [code] BBCode emits <pre><code>, buttons built in JS
#9333	wave 1, part 2 — maintenance tasks as one pick-one form
#9334	wave 1, part 3 — drop the unused jQuery Custom Scrollbar
#9335	wave 1, part 4 — replace the jQuery At.js and Caret.js plugins
#9337	wave 1, part 5 — autosave drafts from an SCEditor plugin
#9338	wave 1, part 6 — repair the post preview

They were tested together, not only one at a time: I keep a branch of release-3.0 with all six merged and re-run every part's checks on it. The combined editor comes up with plugins: "smf,drafts,messageDrafts,mentions,autolinker", and code blocks, mentions, drafts and preview all work alongside each other.

One overlap to know about at merge time: #9337 and #9335 both add to the editor's plugins option in loadEditor(), so whichever lands second needs a conflict resolved. It is a union — both if blocks appending to $plugins. The exact resolution is written out on #9337, and I have built and tested that combination.

The thing I did not expect

Four of the six parts turned out to be repairs, not moves. These are all broken on release-3.0 today:

• The error log's Select and Expand buttons. Errors.template.php builds its own code block by hand, and this branch never updated it.
• The mentions dropdown. The ported At.js render() loops to the configured limit rather than the number of results, so it threw before reaching show() — the list was built and highlighted but never displayed, for any search narrow enough to match fewer than ten members.
• Draft autosave. drafts.js calls sceditor.instance(...).getText(), which the bundled SCEditor no longer has. Nothing has been saved for a while; smf_user_drafts stays empty and id_draft stays 0.
• Post preview. Same missing getText(), plus oEditorHandle_<id>, which no longer exists but is still assigned in the post and topic templates — that throws and abandons the rest of the inline script.

None of it was findable in the combined diff. That is the clearest argument I have for doing the split this way.

Five parts dropped, with reasons

I would rather not carry these forward:

• Self-hosting FontAwesome — the eight webfont files this branch adds are referenced by nothing; its own fontawesome.min.css has no @font-face rules.
• Moving JumpTo into script.js — release-3.0 already moved JumpTo and IconList the other way in d591cd6, and has added Ctrl+Enter support and a redirect fix since.
• The quoteFast plugin — quoting works today. The plugin calls insertQuoteFast unconditionally, dropping the case where the quick reply is collapsed and the click should take you to the full post page instead, and it finds the quote link at el.children[iChildNum].firstElementChild, a fixed position that shifts as soon as a button is hidden by permissions.
• The xmlPreview plugin — it captures new FormData(form) once at signalReady and reuses it for every click, so the preview would show the form as it was at page load rather than what was typed. I repaired the existing preview code instead, in [3.0] Theme split (wave 1, part 6) — Repair the post preview #9338.
• The remaining Editor.php and sceditor.plugins.smf.js changes — 114 of the 151 changed lines in Editor.php are docblock. Of the rest, most is cosmetic, or tied to the two plugins above, or user-visible toolbar changes such as removing the spoiler and details buttons, which belong with the editor's visual rework rather than here. One that looks like an improvement is not: the new autolinker guard calls allowedTo('bbc_url'), and no such permission exists in release-3.0, so the autolinker would simply stop loading.

If you would rather any of these were ported as-is and the problems raised separately, say so and I will.

One bug that needs a home

Updating an existing draft never writes. The client sends the right request and the server answers with a fresh "last saved" time, but the row does not change. In the same request the database layer can SELECT the row by that id, yet the UPDATE straight afterwards reports pg_affected_rows() == 0 and the row keeps its old body and poster_time. The same statement run by hand updates it fine, and Draft::saveToDatabase() returns true either way, so nothing surfaces.

That is server-side and has nothing to do with the theme. It was simply invisible until autosave got far enough to save a first draft. Right now it is only written up inside #9337, so it will be lost if that PR is reworked — it should probably be its own issue. Happy to open one.

Where that leaves things

Wave 2 is the CSS foundation: design tokens and variables.css first, then dark.css, variants, logical properties and icons.css. Tokens gate almost everything after them, so that part is on the critical path for the rest of the series.

The questions from my last comment are still open, and the one that matters most now is whether fixes like the four above should keep going inside the split PRs, or be split out as bug PRs against release-3.0 so the series stays a pure move. Four of six parts landing as repairs makes that a bigger decision than it looked when I first asked.

Happy to start wave 2, or to hold until you have looked at what is open.

[live627]

Here's a cleaner version of the ending that reads a bit more naturally while keeping the same intent:

1. Does the grouping work for you, or would you rather have it carved differently before I go further?

Works for me, but I'd like confirmation from @Sesquipedalian, as he is the lead developer, although I might end up being the one to review and merge these.

2. For the editor and drafts: subdivide as above and carry the fixes inside those PRs, or land the port as-is and raise the defects separately?

• Fixes from release-3.0 go into their own PRs.
• Subdivide with fixes from theme.
• Recent editor changes in Rewrite our editor integration #8949 include many editor changes from this theme branch, except for the plugins.

3. Should fixes like these keep going into the split PRs, or would you prefer them as standalone bug PRs against release-3.0 so the split stays a pure move?

• These fixes can keep going into the split PRs.
• Fixes from release-3.0 go into their own PRs.

4. Happy for me to start wave 2 (the CSS foundation), or would you rather review what is open first? Design tokens gate most of what follows, so that one is on the critical path.

Let's wait on wave 2 until wave 1 has been reviewed and merged.

@albertlast, thanks for taking this on, very helpful, ja?

@jdarwood007, let's consider this branch closed. Please don't make any further commits to it; any additional work should go into the split PRs instead.