Admin: rich Bootstrap 3 renderer for the System Docs help pages#5596
Admin: rich Bootstrap 3 renderer for the System Docs help pages#5596Kanonimpresor wants to merge 3 commits into
Conversation
![e107help[bot]](https://avatars.githubusercontent.com/in/61867?s=60&v=4){kind=small}
There was a problem hiding this comment.
Olá, @Kanonimpresor, thanks for putting this together. The old Q>/A> parser was really showing its age, and having callouts, figures, and step lists first-class is a proper lift.
Four things worth flagging before it lands:
1. Dark-skin breakage. Most of the inlined CSS at e107_admin/docs.php#L29-L61 hard-codes light colours (#555, #777, #fafafa), which go unreadable on the default admin skin. After a core update, admincss is set to css/modern-dark-2.css by e107_admin/update_routines.php#L649-L651; that skin renders body { color:#c6c6c6; background-color:#262626 }. Details in the inline comment.
2. Multi-line A> regression. The new per-line regex at e107_admin/docs.php#L278 only captures the first line after A>. Master's parser used preg_replace('/Q\>(.*?)A>/si', ...), which crossed newlines. For English you worked around it by rewriting multi-line answers as STEP>, but any third-party language or site-local edit still using legacy multi-line A> will render the panel body empty and leak the rest of the answer into plain paragraphs outside the panel. This contradicts "Other languages keep working as-is" in the PR body.
3. "Preserving every original Q/A" isn't quite what happened. Spot-checking Administrators: the original A> "These allow you to give administrators…" is rewritten to "Permissions allow you to give administrators…", and the original second Q ("I want to make an existing registered member into an administrator, how?") is dropped entirely, with its answer restructured into STEP>s. The rewrite itself reads better; worth updating the PR description so reviewers know the copy was paraphrased rather than preserved verbatim.
4. Backwards compatibility. Upstream e107 is two decades old and supports sites running skins untouched for 10+ years. The CSS bit is the urgent one; the A> parser change is the sneakier one, because it only breaks content that doesn't live in this repo.
A handful of smaller things inline: an invalid-HTML edge case in the TOC builder, a PHP 8 warning on empty IMG>, an unescaped title interpolation, a brittle Q/A sentinel, and a pass on comments that don't earn their keep.
Repo-weight note while I'm here: e107_docs/help/_media/ ships 4.4 MB across 16 JPGs, with news/newspost-admin.jpg alone at 703 KB. A lossy pass or a smaller target width would typically shave 50-80% off admin captures like these without the reader noticing. Not blocking, just flagging for the history.
Standard caveat, I'm a bot. @Deltik has final say on merge.
Addresses the eight review points raised on commit 6f61b61 by the e107help bot review (e107inc#5596 review #4147770569). All changes are contained to e107_admin/docs.php. 1) Multi-line A> regression (and the panel-body sentinel that drove it) pattern with a small state machine that buffers the answer body until the next marker, blank line or EOF and then emits the whole panel (heading + buffered body + closing tags) as one unit. Restores the master parser contract 'Q\>(.*?)A>' for any third-party language or site-local file still relying on multi-line A>. Same buffer lets P>, NOTE>, TIP>, WARN> and A> span multiple lines, which reads better for longer callouts. 2) Orphan Q> leaking unclosed panel/panel-collapse/panel-body With the sentinel pattern gone, an orphan Q> now flushes through flushPendingQuestion() which emits a complete (empty-body) panel before closeFaq() runs. Previous code only closed panel-group and left N-1 panels unclosed when several orphans appeared. 3) slug() collisions on duplicate H1>/H2> text Added a per-doc $seenSlugs map and a $uniqueSlug() closure that appends -2, -3, … on collision. TOC links now resolve to the right heading and the rendered HTML validates. 4) resolveMedia() PHP 8 warning on empty IMG>/SHOT> Empty payload now returns '' before reaching $src[0], so a stray IMG> or SHOT> no longer fills the error log with 'Uninitialized string offset 0'. The IMG/SHOT case also short-circuits the figure emission when the source is empty. 5) buildToc() invalid HTML when H1>/H2> are mixed Nested <ul> is now opened inside the still-open <li> rather than as its sibling, and stepping back out closes </li></ul> in the right order. Tested with [H1, H2, H1] and [H1, H2, H2, H1]; result is well-formed and assistive tech announces the outline correctly. 6) Unescaped $title interpolation in the doc wrapper $title (filename-derived) is now passed through htmlspecialchars() before going into the <h2 class='docs-title'> output, matching the escaping already done a few lines up for the figure caption. 7) Inline CSS preamble comment trimmed to one line The 6-line explanatory block above e107::css('inline', …) is now a single-line note about the structure-vs-palette split. The other two per-block intros the bot flagged are no longer present after the sentinel/regex refactor. The renderDoc() refactor also keeps a continuation buffer for plain lines between Q> and A>, so legacy files that wrote the question across two lines still render correctly even though that wasn't on the bot's list.
|
Right, follow-up note on the round-2 review. While applying @Deltik's fix for the @Kanonimpresor, proper turnaround on round 1. Every item from that pass landed in Round-1 follow-ups, verified at
The One new bug from this pass, plus housekeeping:
Repo weight (24 MB). Separate question for you @Kanonimpresor and @Deltik. The transparent-PNG conversion is the right call for the dark-skin readability story, but it ballooned the tarball from ~15 MB on master to ~39 MB on this branch. A direction that I think makes everyone happy, if you're up for it:
If you're game, I can file a tracking issue for the auto-generation side too. Deltik's been kicking around the idea of pixel-diff acceptance tests that would render the admin pages headlessly and dump the captures into the blobs repo automatically. Long-term that takes the screenshot-maintenance burden off contributors entirely. Only if you're comfortable. Say the word and I'll open it as an exploration item with you and @Deltik on the Cc. Tests, a soft offer not a gate. The state-machine refactor is exactly the kind of change a few small fixtures would lock in for next time. I sketched a regression test for the multi-line |
Squashed rebase of feat/admin-docs-rich-renderer onto current master,
incorporating both review rounds.
== Renderer (e107_admin/docs.php) ==
New line-based parser inside docs_ui::renderDoc() that keeps full
backward compatibility with the legacy Q>/A> markup but adds:
H1> / H2> section headings (auto-anchored, populate the TOC)
P> lead paragraph (multi-line)
NOTE> / TIP> / WARN>
Bootstrap-style callouts with FA icons (multi-line)
IMG> / SHOT> figures; SHOT> resolves against e107_docs/help/_media/
STEP> numbered steps grouped in <ol class='docs-steps'>
CODE> syntax-highlighted code blocks
- Free text passes through e107::getParser()->toHTML() so BBCode
([b], [url], [img], ...) works inside help files.
- Q>/A> pairs render as Bootstrap 3 panel-collapse accordions.
- Auto-generated TOC (floating .docs-toc aside) from H1>/H2>.
- Footer-inline JS adds smooth scroll for TOC links.
- Chevron animation owned entirely by Bootstrap collapse.data-api;
no inline handler fights it (round-2 fix).
State machine ( / / flushPendingQuestion() /
flushPendingBlock() / emitPanel()) replaces the original sentinel
approach so multi-line A> bodies stay inside the panel-body wrapper
and orphan Q> entries flush as empty-body panels rather than leaking
into trailing paragraphs.
Other hardening (round 1 + 2):
- slug() de-duplicates with a uniqueSlug closure + $seenSlugs map
(-2, -3 suffixes) so repeated headings get distinct anchors.
- resolveMedia() short-circuits on empty input (PHP 8 warning fix)
and only resolves IMG/SHOT lines.
- buildToc() opens nested <ul> inside the still-open <li>; verified
well-formed for [H1,H2,H1] and [H1,H2,H2,H1].
- $title interpolation passes through htmlspecialchars(ENT_QUOTES).
== Skin-aware styling ==
Inline CSS in docs.php contains structure only (margins, padding,
flex). Per-skin .docs-item palette lives in each of the six bundled
admin skins so the default modern-dark-2 (set by post-install update
routine 649-651) and the rest stay readable:
e107_themes/bootstrap3/css/{kadmin,corporate,modern-light,
modern-dark,modern-dark-2,
bootstrap-dark.min}.css
Callouts use native Bootstrap .alert-info / .alert-success /
.alert-warning so the CDN-hosted skins (Flatly, Sandstone, Superhero)
inherit themed colours for free.
== Content ==
All 21 English help files rewritten with the new markers. Most Q/A
pairs preserved; a few were paraphrased or restructured into STEP>
sequences for readability.
New shared screenshot folder e107_docs/help/_media/<topic>/ ships 16
real captures plus README.md and SCREENSHOTS.md sidecars.
== Compatibility ==
- Other languages keep working: parser falls back to plain
paragraphs + BBCode for everything outside the new markers.
- No DB schema changes, no new dependencies.
- Path constants and coreLan('docs') calls untouched.
== Testing ==
- php -l on docs.php: clean.
- Manual: each sidebar entry renders TOC + collapsible FAQs +
captioned screenshots + correctly-coloured callouts.
- Verified on PHP 8.x.
Squashes 6f61b61 (initial), 1937e77 (round-1 Deltik response),
f22d594 (round-1 bot response), 275386d (screenshot rename),
5f650fd (round-2 bot response: chevron + SCREENSHOTS.md).
Kanonimpresor
force-pushed
the
feat/admin-docs-rich-renderer
branch
from
5f650fd to
e097e4b
Compare
Re-encoded the 16 transparent PNGs under e107_docs/help/_media/ with a quality pass that preserves visual fidelity but cuts the on-disk size from ~25 MB to ~6.7 MB (-73%). Largest single saving: news/admin-newspost.png shrinks from 6.2 MB to ~340 KB (-95%) after a sane resize from 6752x4000 to a docs-appropriate width. Addresses the repo-weight concern raised by e107help[bot] in the round-2 review on PR e107inc#5596 and echoed by @Jimmi08 in discussion e107inc#5600. PNG transparency preserved so the captures stay readable on every admin skin (light, dark, modern-dark-2).
Locks in the contracts established by docs_ui::renderDoc() / buildToc() / slug() so any future change that breaks them fails fast in CI. Four fixtures, mirroring the bot's round-2 sketch on PR e107inc#5596: - testMultilineAnswerStaysInsidePanelBody: multi-line A> body must stay inside <div class='panel-body'>...</div>, matching the legacy /Q\>(.*?)A>/si contract for translations and site-local docs. - testOrphanQuestionEmitsEmptyPanel: a Q> with no trailing A> must emit a closed empty-body panel; a following P> must NOT be captured inside that panel-body. - testRepeatedHeadingsGetUniqueSlugs: two H1> headings with identical text must get distinct anchors (base, base-2) and both must appear in the TOC. - testBuildTocProducesValidNestedHtml: nested <ul> may only appear inside a still-open <li>, never as a direct sibling; <ul>/</ul> and <li>/</li> tags must balance for a mixed [H1, H2, H1] heading sequence. Approach: docs.php is a full admin entry point (requires class2.php, exits unless ADMIN, instantiates the dispatcher), so the test extracts the docs_ui / docs_form_ui class bodies with a regex and eval()s them into the symbol table once. e_admin_ui (the parent) is required first; ReflectionClass::newInstanceWithoutConstructor() then sidesteps the e_admin_ui chain so we can call private renderDoc() / buildToc() / slug() in isolation. Same \Codeception\Test\Unit shape as e107_tests/tests/unit/CronParserTest.php.
|
@e107help — round 2 follow-ups all addressed. Three new commits on top of the squash: e097e4b — squashed rebase onto current master (incorporates round 1 + round 2 fixes from the prior commit chain). Resolved 7 conflicts from #5595 (the six bootstrap3 admin skins + docs.php); the skin palettes were purely additive on top of the phpinfo block, and the docs.php hunks took the renderer side cleanly. fbaeed3 — _media re-encoded with a quality pass: ~25 MB → ~6.7 MB (-73%). Largest single saving: news/admin-newspost.png from 6.2 MB → ~340 KB (-95%) after a sane resize from 6752×4000 to a docs-appropriate width. Transparency preserved so the captures stay readable on every admin skin (light, dark, modern-dark-2). Tarball impact for this PR drops from "+24 MB on top of master" to "+6.7 MB on top of master". This addresses the repo-weight concern surfaced here and echoed by @Jimmi08 in #5600 — the media-repo + jsDelivr move you proposed remains a sensible follow-up if @Deltik wants to push further, but it's no longer urgent. 05357eb — e107_tests/tests/unit/DocsRendererTest.php with the four fixtures from your sketch: testMultilineAnswerStaysInsidePanelBody — locks the legacy Q>(.*?)A> contract for translations and site-local docs.
Approach: docs.php is a full admin entry point (requires class2.php, exits unless ADMIN, instantiates the dispatcher), so the test extracts docs_ui / docs_form_ui with a regex and eval()s the bodies once into the symbol table, then reaches renderDoc() / buildToc() / slug() via ReflectionClass::newInstanceWithoutConstructor() to skip the e_admin_ui chain. Same \Codeception\Test\Unit shape as CronParserTest.php. Round-2 inline items also done: Chevron handler at docs.php#L74-L76 deleted in this round (was already in 5f650fd, retained in the squash). Bootstrap 3's collapse.data-api now owns .collapsed exclusively, so the transition fires cleanly on first click and stays in phase on subsequent clicks. Diagnosis matched yours exactly. cc @Deltik — ready for another look whenever you have a window. |
|
Right, went through the three new commits, @Kanonimpresor. They all hold up.
The part that actually makes me smile: the test file landed. On repo weight, +6.7 MB on top of master is small enough that the sibling-repo + jsDelivr idea is properly a "nice to have if @Deltik wants to push further", not urgent any more. The cross-link with #5600 still holds as direction; I'm happy to draft a tracking issue for that whenever a rainy day rolls round. Three threads on my round-1 review are still technically open: two already marked outdated (chevron drop, SCREENSHOTS.md) and the test-scaffold one is effectively landed in its own file. Give me a nod and I'll resolve all three so the conversation tab quiets down. Merge call is @Deltik's, as always. From my side, this reads ready. |
PR #5596 (feat/admin-docs-rich-renderer) rewrites all 21 English help files in a new line-based format. - Administrators: the "set their permissions" sentence is replaced by a STEP> list under the rewrite, so the line my prior commit edited no longer exists in #5596's tree. - Forums: the same Q/A line is preserved verbatim and already reads "but their name isn't in the list" in #5596's version, making the edit redundant and only adding a nuisance dash-character conflict. Dropping both. lan_prefs.php (PRFLAN_38, PRFLAN_91) stays; #5596 does not touch e107_languages/.
2 participants
Problem
The admin "System Docs" page (
e107_admin/docs.php) injected eachhelp file raw into the DOM, so administrators could not embed
images, lists or callouts. The only formatting was a hard-coded
Q>/A>regex.Changes
docs_ui::renderDoc()that keepsfull backward compatibility with the legacy
Q>/A>markup butadds these markers:
H1>/H2>– section headings (auto-anchored, populate the TOC)P>– lead paragraphNOTE>/TIP>/WARN>– Bootstrap-style callouts with FA iconsIMG>/SHOT>– figures (SHOT>resolves againste107_docs/help/_media/)STEP>– numbered steps grouped in<ol class="docs-steps">CODE>– syntax-highlighted code blockse107::getParser()->toHTML()soBBCode (
[b],[url],[img], …) works inside help files.Q>/A>pairs render as Bootstrap 3panel-collapseaccordions..docs-tocaside) fromH1>/H2>.footer-inline) adds smooth scroll for TOC links andchevron toggle for the FAQ accordion.
Content migration
All 21 English help files have been rewritten using the new markers. Most original Q/A pairs are preserved verbatim; a few were paraphrased or restructured into STEP> sequences for readability.
New shared screenshot folder
e107_docs/help/_media/<topic>/ships with 16 real captures.Compatibility
paragraphs + BBCode for everything outside the new markers).
coreLan('docs')calls untouched.Testing
e107_admin/docs.php, click each entry in the sidebar:TOC appears, sections collapse/expand, screenshots render with
caption, callouts get the right colour and icon.
php -l(no syntax errors) and PHP 8.x.