fix(ui): name every bundled Bootstrap on core's right floats - #6449
Open
e107help[bot] wants to merge 2 commits into
Open
e107help[bot] wants to merge 2 commits into
e107help[bot] wants to merge 2 commits into
Conversation
…ventions tests The dropdown-menu conventions test walks core's PHP, reads every class attribute through the tokeniser and keys each list by where it was found, and the right-float conventions test that follows wants the same walk over the same files. Test\Markup holds the one copy and each test filters what comes back, beside Test\Tree, which already holds the file list that feeds it. Keeping what each file read costs a little memory and saves the second rule a whole pass over the tree. Test\Tree also learns which of its trees hold core's own code, so a rule that stops at the bundled themes asks it for corePhpFiles() instead of repeating the directory name the class already carries.
Core bundles Bootstrap 3, 4 and 5, and each spells float:right !important differently: pull-right, float-right and float-end respectively. None of the three reads either of the others, so markup naming one of them stops floating on a theme that loads a different one, and a control meant to sit at the right edge drops back into the flow. Twenty-one class lists on release/v2.3.x already carry all three and #6431 brings two more; this puts the rest of core on the same list. This is the twin of the same change on master, #6448. Core's own .f-right in e107_web/css/e107.css looks like the tidier answer to "float right whatever the theme loads" and is deliberately not what is used. The three vendor spellings are float:right !important and .f-right is not, so a framework rule on the same box beats one and not the other. The custom page navigation wrapper already carries .f-right and is completed anyway rather than carved out. The forum's post-options menu goes the other way, because there the float was the defect. Its ten entries are each an <li> inside <ul class="dropdown-menu"> carrying text-right text-end float-right, and Bootstrap 4 is the one framework that defines float-right. Measured against the bundled Bootstrap 4 stylesheet, a 160px menu draws its entries 117px, 110px and 87px wide instead of 158px, so only the label is clickable, and the separator between the two groups leaves the flow the floats took over and is drawn beside the first entry rather than between the groups. text-right text-end is what right-aligns the labels and it is already there, so float-right comes off rather than being completed. One of them named it twice into the bargain. The hero media wrapper is the one addition that changes what a modern theme draws. The media is a 400x400 thumbnail, fixed by the {SETIMAGE: w=400&h=400} at the top of the same template, it sits alone in a half-width column, and hero.css insets that column 45px on the right, so a thumbnail held at the column's right edge is what the plugin was written to. Bootstrap 3 draws that today and 4 and 5 do not, because neither reads pull-right; there the w-100 on the image stretches a 400px thumbnail across the column instead. Completing the list gives all three the same thumbnail at its own size where the stylesheet expects it. The column holds nothing but that wrapper and is a block formatting context in all three frameworks, a float on 3 and a flex item on 4 and 5, so its height still follows the image. The forum search box is the one addition live on Bootstrap 3 alone: it named 4's and 5's spellings and not 3's, and the .right beside them is core's own text-align:right rather than a float, so on a legacy theme the box spanned the row instead of sitting at the right of it. It now floats there as it already does on 4 and 5, and the table below it clears under it as it already does there. Several completions are inert where they land. The installer links the bundled bootstrap3 stylesheets and no others; no bundled theme gives the admin area anything but Bootstrap 3; and the two Who's Online entries are flex items, where float is ignored. The menu-manager area selector is the case #6328 left at pull-right, because bootstrap3/admin_style.css pins it position:absolute and float computes to none on an absolutely positioned box. All of them are completed regardless: that pin is one admin skin's rather than core's, and a mechanical rule with sites carved out of it does not survive the next person to read it. e107_themes is left alone and the conventions test scopes itself to the trees core ships its own code in. A theme declares the one Bootstrap it loads, so naming that version's spelling alone is honest there in a way it is not in core. The six lines concerned are Jimmi08's call and she has been asked on the issue. Sweeping every site at once rather than one at a time is the ruling on #6328 applied to the float list. None of the six plugins this touches has been bumped since v2.3.12, so the rule bumps all six. chatbox_menu, gallery and hero take the numbers master's twin gives them, because on both branches those numbers stand for this change and nothing else. forum and pm take a date move alone, so that both branches ship the same version for the same plugin: bumping them here and not on master would leave this branch ahead, and a site later moving to a master build would read the number going backwards as nothing to upgrade. featurebox goes to 1.1.1 rather than master's 1.2. On master 1.2 stands for the sef migration in 228e391, which this branch does not carry, and a site that took 1.2 here would be sitting on the number that migration uses to announce itself. plugin_class.php compares with a strict less-than and has no branch for an installed version above the file, so that migration could never prompt again. 1.1.1 raises the prompt this change owes and leaves 1.2 for the backport that earns it; list_new already carries a three-part version. online ships no plugin.xml. Refs #6425
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
1 participant
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.
Why
Refs #6425. The release/v2.3.x twin of #6448, which carries the full reasoning.
This branch bundles Bootstrap 3, 4 and 5 as master does, and each declares
float:right !importantunder a name the other two do not define:pull-right,float-rightandfloat-end. Core markup naming one of them stops floating the moment the theme loads a framework it did not name. Twenty-one class lists here already carry all three, which is the house answer; this puts the rest of core on it.Stacked on #6431 for the shared test support, so GitHub will retarget this to
release/v2.3.xwhen that merges.What Changed
A cherry-pick of master's two commits. The scan finds the same thirty-six incomplete class lists here as on master, and the same forty-nine complete ones afterwards, with the class strings character-identical between the branches.
Two conflicts, both resolved by hand:
e107_plugins/forum/shortcodes/batch/view_shortcodes.phpguards the delete-own-post entry with a nested condition and comment that master no longer has, because a different change flattened it there. This branch's condition and comment are kept and only the<li>class list inside them is edited. The!defset('MODERATOR')term master gained is a separate backport and is deliberately not dragged in here.e107_plugins/featurebox/plugin.xml, covered below.The new conventions test extends
\Codeception\Test\Unithere, which is what this branch's unit tests use; master's sharedTest\Unitbase does not exist on it.Plugin versions
None of the six plugins this touches has been bumped since v2.3.12, so the rule bumps all six.
chatbox_menu,galleryandherotake the same numbers master's twin gives them, because on both branches those numbers stand for this change and nothing else.forumandpmtake a date move alone, so that both branches ship the same version for the same plugin; bumping them here and not on master would leave this branch ahead, and a site later moving to a master build would read the version going backwards as nothing to upgrade.featureboxis the one departure and is worth a second look before merging. It goes to 1.1.1, not master's 1.2. On master that 1.2 was set by228e391e92, the sef migration, whose own message says the bump exists to raise the update prompt. A site that took 1.2 from this branch would be sitting on the number that migration announces itself with, andplugin_class.php:1661compares with a strict less-than and has no branch for an installed version above the file, so the migration could never prompt again. 1.1.1 raises the prompt this change owes and leaves 1.2 for the backport that earns it.list_newalready ships a three-part version, so the shape is not new here.How It Was Tested
The whole unit suite passes on this branch, on PHP 8 and again on a PHP 5.6 environment: 1539 tests, 19502 assertions. The whole tree parses under real PHP 5.6, and
php -lpasses on all 31 changed files.e107_tests/tests/unit/RightFloatConventionsTest.phpreads every class attribute core writes throughtoken_get_all()and fails any list naming one of the three spellings without naming all three. It reds on the unfixed tree and greens with the change.Backwards Compatibility
As on #6448, and the same three deliberate movements: the ten forum post-options entries stop floating on Bootstrap 4, the forum search box begins floating on Bootstrap 3, and the hero media wrapper begins floating on Bootstrap 4 and 5. Everything else adds tokens that the framework in play does not define, so the page is unchanged in effect.
AI Model
Claude Opus 5 (claude-opus-5), as e107help.
Checklist