[3.0] Theme split (wave 8, part 5) — logical margins and padding where the sides mirror - #9572
Open
albertlast wants to merge 5 commits into
Open
[3.0] Theme split (wave 8, part 5) — logical margins and padding where the sides mirror#9572albertlast wants to merge 5 commits into
albertlast wants to merge 5 commits into
Conversation
rtl.css exists to flip physical properties for right-to-left languages, so every rule in it is an override of something another stylesheet draws. Sixteen of them override markup the theme stopped emitting, and have been dead for some time: - the event editor was rewritten and now draws no ul at all, which takes out the whole "edit event section" block plus the two #event_main rules - .bbc_standard_quote and .bbc_alternate_quote were checked and kept: those class names are built at runtime in BBCodeParser, so grepping for them finds nothing even though the markup does emit them - the rest name ids and classes that appear nowhere in Themes/ or Sources/: #postbuttons_upper, .reportlinks, #personal_messages span#author and span#topic_title, h3.search_hd, #popularposts, #popularactivity, .topic .mod_icons, .stats_icon, .search_results_posts .buttons and #helpmain h3.section Checked by loading 24 pages as an administrator with a right-to-left language active, including the event editor, the statistics centre, help, search results and the personal messages area: 7086 elements, and not one of the removed selectors matches any of them. .topic .mod_icons is the counterpart of the index.css rule removed in SimpleMachines#9547, which left this copy behind. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
float: inline-start and float: inline-end follow the writing direction on their own, so a rule that uses them needs no right-to-left counterpart. This converts the 26 floats where rtl.css was doing nothing more than mirroring the value, and drops those overrides. The 26 are the ones where the override is an exact mirror: index.css and profile.css say left and rtl.css says right, or the other way round. Floats whose right-to-left value is not the mirror of the left-to-right one are left alone, because a single logical declaration cannot reproduce two different values; the same goes for the floats rtl.css adds where the base sheet has none. #post_header dt/dd shared a rule with #event_options dt/dd, and only the first pair has a mirror in rtl.css. Converting the pair together would have started mirroring the event editor, which is not mirrored today, so the rule is split and #event_options keeps a physical float with a comment saying why. Checked by recording float, clear, text-align, the inline margins and padding, and the bounding rectangle of every element on 26 pages, before and after, captured back to back so nothing else about the forum could move in between: - left-to-right: 8018 elements, the only differences are the 82 float keywords themselves. No geometry changes at all. - right-to-left: 8017 elements, 81 float keywords, and again nothing else. The keyword is expected to differ: getComputedStyle reports inline-start rather than resolving it to left or right. What matters is that every x, y, width, height, margin and padding is identical in both directions. rtl.css goes from 555 lines to 480. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
text-align: start and text-align: end follow the writing direction, so the eleven places where rtl.css did nothing but mirror the alignment no longer need an override. As with the floats, only exact mirrors are converted: the base sheet says left and rtl.css says right, or the reverse. Alignments that rtl.css sets where the base sheet has none are left alone, and so are the deliberate ones - code blocks and file contents stay left-aligned in a right-to-left language on purpose, and that rule is untouched. Two rules needed care rather than a substitution: - #main_grid td.days shares its rule with #month_grid td.days, but the mini calendar re-declares text-align: center immediately below, so the shared value never reaches it and the group is safe to convert. - rtl.css aligned #stats tr.windowbg th.lefttext and #stats tr.titlebg th.lefttext together, and only the windowbg half has a rule to mirror. The titlebg half is aligned by .lefttext instead, so it keeps its override, now with a comment saying why it is on its own. Checked the same way as the floats, capturing every element on 28 pages before and after, back to back, with text-align and direction recorded alongside the geometry: - left-to-right: 8996 elements, 119 differences, every one the text-align keyword - right-to-left: 8995 elements, 119 differences, likewise No geometry moved in either direction. The count is higher than the eleven converted rules because text-align inherits, so the children report the new keyword too. rtl.css goes from 480 lines to 453. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
clear: inline-start and clear: inline-end follow the writing direction, so the three places where rtl.css only mirrored the side no longer need an override. clear: both is unaffected and untouched. One group is deliberately left alone. index.css clears .signature, .attachments, .under_message, .custom_fields_above_signature and .custom_fields_below_signature together, but rtl.css mirrors only the first, second and fourth of those. Converting the group would start mirroring .under_message and .custom_fields_below_signature as well, which is a change in behaviour rather than a no-op, so the group keeps its physical value. Whether the two missing from rtl.css are an oversight is worth deciding separately - all three appear in the markup, so it is not a question about dead code. Checked as before, on 29 pages captured back to back. A poll was posted and voted on first, because #poll_options dl.options only renders once results are visible and none of the existing pages reached it: - left-to-right: 9735 elements, 5 differences, all the clear keyword - right-to-left: 9735 elements, 5 differences, all the clear keyword No geometry moved in either direction. rtl.css goes from 453 lines to 442. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
margin-inline and padding-inline follow the writing direction, so the twenty rules where rtl.css did nothing but swap the left and right values no longer need an override. A rule qualifies only when the right-to-left values are the exact mirror of the left-to-right ones: the same top and bottom, and the left and right swapped. Four-value shorthands become a block pair and an inline pair, so margin: 0 0 4px 2% turns into margin-block: 0 4px and margin-inline: 2% 0. Where the base sheet used a single longhand it stays a single longhand, so margin-left becomes margin-inline-start. Everything else is left as it is. rtl.css contains a good many spacing rules that are not mirrors at all - they change the value as well as the side, or they add spacing the base sheet does not have - and no logical declaration can express two different values at once. Checked as before, on 29 pages captured back to back: - left-to-right: 9735 elements, 0 differences - right-to-left: 9736 elements, 0 differences Nothing at all changed this time, not even a keyword: getComputedStyle resolves logical margins and padding back to physical ones, so the values it reports are identical before and after. rtl.css goes from 442 lines to 365. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part 5 of wave 8 of the #7933 split. Stacked on #9571, and through it on #9570, #9569
and #9568.
margin-inlineandpadding-inlinefollow the writing direction, so the twenty ruleswhere
rtl.cssdid nothing but swap the left and right values no longer need an override.This is the largest single reduction in the wave.
What qualifies
A rule qualifies only when the right-to-left values are the exact mirror of the
left-to-right ones — same top and bottom, left and right swapped.
margin: 0 0 4px 2%becomesmargin-block: 0 4pxandmargin-inline: 2% 0margin-leftbecomesmargin-inline-startEverything else is left alone.
rtl.cssholds a good many spacing rules that are notmirrors — they change the value as well as the side (
.cat_bar .descis8pxon one sidein one direction and
13pxin the other), or they add spacing the base sheet does not haveat all. No logical declaration can express two different values at once, so those stay.
How this was checked
Same method as parts 2 to 4, on 29 pages captured back to back across a stash.
Nothing changed at all this time — not even a keyword. Unlike
floatandtext-align,getComputedStyleresolves logical margins and padding back to physical values, so thereported numbers are identical before and after. This is the strongest result of the wave:
the two stylesheets are indistinguishable to the browser.
One measurement note, since it would otherwise look like a real difference. A first run
showed four differences on the statistics page, all on the counter
<span>s insidedl.stats dd. That was the counters themselves changing value between runs — the text gotone character wider — and not the CSS. Two captures taken with identical code showed the
same four, and re-running the pair once the counters settled gave the zero above.
rtl.cssgoes from 442 lines to 365 — down from 624 at the start of the wave.Issues References (Fixes|Related|Closes)
Related to #7933.