Skip to content

[3.0] Theme split (wave 2, all parts) — CSS foundation, as one PR instead of eight - #9358

Closed
albertlast wants to merge 16 commits into
SimpleMachines:release-3.0from
albertlast:3.0/theme-wave-2
Closed

[3.0] Theme split (wave 2, all parts) — CSS foundation, as one PR instead of eight#9358
albertlast wants to merge 16 commits into
SimpleMachines:release-3.0from
albertlast:3.0/theme-wave-2

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

This is an alternative, not an addition. It is the whole of wave 2 in one place, for anyone who would rather take it as a single unit than as eight. Merging this means closing #9350, #9351, #9352, #9353, #9354, #9355, #9356 and #9357; merging those means closing this. Do not merge both.

Description

Wave 2 of the #7933 split: the CSS foundation. Eight parts, each of which is also its own PR if you would rather review them separately.

Part What it does PR
1 Adds variables.css; base tokens — html, body, selection, inputs, fieldsets, strong, headings, hr #9350
2 Buttons, quickbuttons and the inline mod check #9351
3 Cat bar, title bar, sub bar, roundframe, information, windowbg and its states #9352
4 Error, notice, info and desc boxes; the generic and progress bars #9353
5 Blockquote, cite, the quote backgrounds, code blocks, BBC links #9354
6 AJAX notification bar, popups, search highlight, moderation link #9355
7 Logical properties for the float and alignment utilities #9356
8 Logical properties for the settings lists #9357

Net effect: variables.css arrives with 185 tokens, index.css gains 159 var() references, and rtl.css drops from 686 to 646 lines.

Two things worth knowing before reading the diff

Theme.php already asked for this file. loadCss() has called loadCSSFile('variables.css', …, 'order_pos' => -2) since 3.0 Alpha, and the default theme has never shipped it. A missing stylesheet is dropped without complaint, so the call has been a no-op all along. No Theme.php change is needed here — order_pos -2 already puts it ahead of index.css at 1. (icons.css is loaded the same way two lines above and is also still missing.)

Every value is the one the rule already had. #7933's variables.css is a redesign, not a re-expression: --body-bg is hsl(207, 45%, 95%) against today's #e9eef2, --body-color #292929 against #4d4d4d, --body-font-size 94% against 83.33%. Taking it as it stands would introduce the token layer and change the look in one diff, which is a large part of what made #7933 hard to review. So this uses #7933's names with today's values. Changing the palette becomes its own PR afterwards, and that diff is nothing but values.

Merge test

All eight branches were merged into one, in order, from a fresh release-3.0. No conflicts. The resulting tree is byte-identical to the linear stack (git diff 3.0/theme-logical-settings 3.0/theme-wave-2 is empty), so the parts compose exactly as each one claimed on its own — nothing compounds.

Testing

Fresh install on MySQL in the Docker environment, combined branch against plain release-3.0, not against the previous part.

Computed styles, 12 pages — board index, a board, a topic, the posting form, search, a profile, admin feature settings, admin maintenance, stats, memberlist, PMs and help — across 46 selectors and 34 properties, with every border side and every corner radius read separately:

232 selector rows, 7888 property comparisons, 29 differences — and all 29 are float: left → inline-start, clear: right → inline-end or text-align: right → end. getComputedStyle returns logical keywords as authored rather than resolving them, so it reports a difference for every converted declaration and settles nothing either way. Not one colour, border, shadow, radius, padding or margin differs.

Geometry, which is what actually settles parts 7 and 8. Each probe records a box's distance from both edges of a fixed 400px parent, its vertical offset, width and height, so a float landing on the wrong side, a clear that fails to clear and an alignment that does not move all show up as numbers. Run with lang_rtl off and on:

probes differences
LTR 19 0
RTL 19 2

The two RTL differences are the repair described in #9356. .righttext and .lefttext set an auto margin as well as a text alignment, and rtl.css only ever flipped the alignment — so in an RTL forum a .righttext block has been hugging the right edge, the start side, while its own text aligned left. The box now sits on the end side, where its text already was. The text alignment itself does not move: the same run measures where text lands inside the block and reports no change in either direction.

Per-part checks are in the individual PRs and are not repeated here: each token was also read back from :root and compared against the literal it replaced, which is what caught --searchhighlight-bg: inherit in #9355 — a custom property cannot hold inherit, and the computed-style probe could not see it.

Error log clean throughout. The unit suite does not reach CSS, so there is no test to add.

Left hard-coded on purpose

Where a rule is shared between two things #7933 gives separate vocabularies to, tokenising it would mean choosing a name, which is a palette decision rather than a rename. Those are called out in the part PRs: .pagesection .button, the #222 in the shared button hover rule, the h3.titlebg / h3.subbg rule, .codeheader / .quoteheader, .errorbox's absent colour, and .highlight's background-color: inherit.

Issues References (Fixes|Related|Closes)

Related to #7933
Alternative to #9350, #9351, #9352, #9353, #9354, #9355, #9356, #9357

Theme::loadCss() has been asking for variables.css since 3.0 Alpha, at
order_pos -2 so it lands ahead of index.css, but the default theme never
shipped the file. The request is quietly dropped, so nothing breaks and
nothing happens either. This adds it.

The first group of tokens covers what index.css sets before it gets to any
particular part of the forum: the html and body backgrounds, the body font
and colour, the selection colours, form controls in their normal, hover,
focus and disabled states, fieldsets and their legends, strong, headings,
and the horizontal rule.

Every value is the one that rule already had, so this changes no pixels.
That is the point: it puts the vocabulary in place, with the names the
new theme uses, so the parts that follow can convert their own section of
index.css without also arguing about what the value ought to be. Changing
what the forum looks like is a separate job from giving its values names.

Verified by comparing computed styles across eight pages - board index,
board, topic, posting form, search, profile, admin and help - for the
eighteen selectors these rules reach and the fourteen properties they set.
1316 comparisons, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Second group of tokens: the .button family, which the quickbuttons and the
inline moderation checkbox share the same rules with. Normal, hover, focus
and active states, plus the border, radius, shadow, cursor, font size, text
transform and the text shadow the active button carries.

As with the first group, every value is the one the rule already had, so
nothing changes on screen. The two hard-coded values left in these rules are
deliberate: .pagesection .button uses the body link colour rather than a
button colour, and the SimpleMachines#222 in the shared hover rule only ever reaches the
quickbuttons, because the very next rule overrides it for .button. Neither
has an honest name in this vocabulary yet.

Verified two ways. Each of the nineteen tokens resolves to exactly the
literal it replaced, read back from :root in the browser. Then computed
styles for eleven button variants - plain, active, quickbutton, inline mod
check, first and last in a buttonrow, inside a cat_bar, smalltext, and the
three of those that can hold focus - across twenty properties. 220
comparisons, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Third group: the containers every page is built out of. The cat bar and its
heading, the title bar, the sub bar, the roundframe, the information box,
and the windowbg together with the backgrounds it takes when a topic is
locked, sticky, both, or waiting for approval.

Same rule as the earlier groups - every value is the one that rule already
had, and nothing changes on screen.

One thing worth knowing for the parts that follow. Three of these rules set
a border on a single side: div.cat_bar and .sub_bar have a border-bottom,
and .information has all four minus the top. Rewriting those as the
border-color / border-style / border-width triple looks equivalent and is
not: it leaves style and colour set on the sides whose width is zero, so a
later rule that gives one of those sides a width would paint a border that
could not have appeared before. The single-side shorthand is kept.

Two values are left hard-coded. The shared rule for h3.titlebg and h3.subbg
sets one colour and one font family for both bars, so tokenising it means
choosing which of the two names it belongs to; that is a decision for
whoever changes the palette. And what the theme calls the even row is this
theme's odd one, so #fdfdfd is --window-odd-bg here rather than borrowing a
name that would then mean the opposite.

Verified the same two ways as the earlier parts. All 48 tokens resolve to
the literal they replaced, and computed styles for 22 container variants
across 23 properties, including all four border widths and colours and each
corner radius separately: 506 comparisons, no differences. The border point
above is not theoretical - the first attempt at these rules produced 14 of
them, which is how it was found.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Fourth group: the boxes SMF talks to people through - error, notice, info
and desc - and the two bars, the generic one used for stats and the
progress bar with its four colours.

Every value is the one the rule already had, so nothing changes on screen.

The error, notice and info boxes each carry a border on the top and bottom
only, so they keep their two single-side shorthands for the reason the
previous part ran into: writing them as border-color, border-style and
border-width would leave a style and a colour on the left and right sides,
where today there is none.

.errorbox sets no colour of its own, so there is no --errorbox-color to add
yet; it inherits, and that is worth keeping visible rather than inventing a
value for it.

Verified as before. All 40 tokens resolve to the literal they replaced, and
computed styles for 23 elements - each box, the error box heading, its alert
paragraph and its span, both bars, their inner bar, the vertical variant and
the four progress colours - across 27 properties, with every border side
read separately. 621 comparisons, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Fifth group: what BBC produces inside a post. Blockquote and its cite, the
standard and alternate quote backgrounds, the code block, and the underline
on a BBC link in both its states.

Every value is the one the rule already had, so nothing changes on screen.

Two of these rules do carry a border on every side, unlike the ones in the
previous parts, so the border-color, border-style and border-width triple is
the right shape for them and reproduces what the shorthands were doing:
blockquote is 1px 2px in #d6dfe2 #aaa, and .bbc_code is 2px 1px 3px in
#bbb #dfdfdf #aaa. The single-side borders on blockquote cite and .bbc_link
keep their shorthand.

.codeheader and .quoteheader share one rule and one set of values, so
tokenising it means choosing whether it belongs to the code vocabulary or
the quote one - the same situation as the title bar and sub bar headings in
the third part, and left the same way, for whoever changes the palette.

Verified as before. All 21 tokens resolve to the literal they replaced, and
computed styles for 11 elements across 23 properties, every border side
read separately: 253 comparisons, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sixth group: the things that sit on top of a page rather than inside it.
The AJAX notification bar and its link, the popup container, window, heading
and content, the search result highlight, and the moderation link.

Every value is the one the rule already had, so nothing changes on screen.

One value is deliberately not a token. .highlight has background-color:
inherit, and a custom property cannot hold that: inherit is a CSS-wide
keyword, so --searchhighlight-bg: inherit on :root asks to inherit from an
element that has no parent, resolves to the guaranteed-invalid value, and
every var() reading it falls back to transparent. Close enough to invisible
in most places to pass a screenshot, and wrong wherever the highlight sits
inside something coloured. It stays a literal, with a comment saying why.

Verified as before, plus one extra check for that case: a .highlight inside
a parent painted rgb(1, 2, 3) still computes to rgb(1, 2, 3) rather than
transparent. All 24 remaining tokens resolve to the literal they replaced,
and computed styles for nine elements across 26 properties: 234
comparisons, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
First of the RTL parts. .floatright, .floatleft, .clear_left, .clear_right,
.righttext, .lefttext and .bbc_list say inline-start and inline-end rather
than left and right, so they follow the writing direction on their own and
rtl.css no longer has to flip them. Seven overrides go with them.

Left to right is untouched: twelve geometry probes, no differences.

Right to left changes in one place, and it is a fix. .righttext and
.lefttext set an auto margin as well as a text alignment, and rtl.css only
ever flipped the alignment. So in an RTL forum a .righttext block has been
hugging the right edge - the start side - while its own text aligned left.
Measured in a 400px container, the box sat 300px from the left before and
sits at 0 now, which is the end side, where its text already was. The
alignment itself does not move: the same probe measuring where the text
lands inside the block reports no change in either direction.

Verified with lang_rtl set on en_US, before and after, on both branches.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
dl.settings is the two column label and value layout that most of the admin
and profile screens are built from. Its clear, the floats on dt, dt.windowbg
and dd, and the gap after an image in a dt are all direction dependent, and
all five had an override in rtl.css.

Saying inline-start and inline-end instead removes the need for them, and
the block of five overrides goes. rtl.css is down to 646 lines from the 686
it started this wave with.

Nothing moves in either direction: seven geometry probes covering the list
itself, both kinds of dt, both dd, the image inside a dt and a list clearing
a float beside it, measured with lang_rtl off and on, no differences.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant