Add <panel> and <section> support for modules - #1
Conversation
- Implemented recursive parsing of PortableInfobox blocks to handle complex nesting. - Added logic to generate Module:Tabber calls for <panel> structures. - Standalone <section> tags are treated as groups. - <label> children are correctly used for tab/section titles and stripped from rows. - Ensured Template output remains unchanged and panels are ignored there. - Fixed UI bug where conversion notes were not displayed. - Added safety checks for MediaWiki globals in standalone environments. - Verified all 10 edge cases including complex nesting and regressions. Co-authored-by: whostacking <144233917+whostacking@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 15 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughAdds a diagnostics notes DOM target and refactors the PortableInfobox parser to emit richer row types (title, image, header, label, section, panel); updates template and module output generators and overhauls Lua rendering to support panels/tabbed sections. ChangesPortableInfobox Parsing and Output Modernization
Sequence Diagram(s)sequenceDiagram
participant Browser
participant readBlocks as readBlocks (tokenizer)
participant parseBlocks as parseBlocks (row ctor)
participant Template as makeTemplateOutput
participant Module as makeModuleOutput / Lua
Browser->>readBlocks: submit infobox source
readBlocks->>parseBlocks: token stream (including panel/section/label & comments)
parseBlocks->>Template: structured rows for template rendering
parseBlocks->>Module: structured rows for module/Lua rendering
Module->>Browser: Lua output (sections, optional Tabber)
Template->>Browser: template wikitext (sectionN / labelN pairs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@index.html`:
- Line 67: The notes list rendered by render() (the <ul id="notesList"> element)
should be exposed as a live region so screen readers announce updates: add
aria-live="polite" and aria-atomic="true" (and optionally role="status") to the
<ul id="notesList"> element so newly populated conversion notes are announced
when render() repopulates the list.
In `@script.js`:
- Around line 164-183: The panel branch only keeps nested <section> blocks and
discards any other direct children, causing loss of rows; update the handler for
tagName === 'panel' to iterate subBlocks (from readBlocks(block.body)) and for
each sub block: if sub.tagName === 'section' build a section using
parseBlocks(sub.body, notes) as before, otherwise treat the sub block as
top-level rows by parsing it with parseBlocks(sub.body, notes) or converting it
into row objects and appending them into the panel (e.g., as an unsectioned
section or merged into sections[].rows), ensuring parseBlocks and label
detection (labelRow) are reused so direct <data>, <header>, <group>, and <label>
children are preserved before pushing the final { type: 'panel', sections } into
rows.
- Around line 366-372: The current code computes titleFallback in JS (using
typeof mw ...) which bakes a concrete string into the generated Lua; instead
build and pass a Lua runtime fallback expression string to luaArg so the Lua
module resolves title at runtime. Locate titleSource and titleFallback around
model.title and replace the JS-evaluated titleFallback with a Lua expression
string that checks args['title'] first, then falls back to evaluating
mw.title.getCurrentTitle().text when mw is available at runtime, and finally a
hardcoded literal like 'Page Title'; keep using luaArg(titleSource,
<lua-fallback-expression>) so infobox:renderHeader receives a Lua-side fallback.
Ensure luaComments and infobox:renderHeader calls remain unchanged.
- Around line 450-453: The header branch currently inserts only row.label into
the infobox (in the block matching row.type === 'header' that calls
table.insert(..., infobox:renderItem({...}) and uses luaString("'''"+ row.label
+"'''")), which drops the source/default fallback chain; replace that direct use
of row.label with the same parameter-resolution/fallback logic used for
top-level headers (i.e., compute the display value by reusing the existing
fallback routine you use elsewhere for headers/parameters) so the value passed
into data = luaString(...) preserves source/default fallbacks before wrapping in
the bold triple-quote.
🪄 Autofix (Beta)
✅ Autofix completed
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- Updated recursive parser to correctly identify and extract section/tab titles from <label> or <header> tags. - Implemented robust support for nested <panel> tags within groups or sections using Lua anonymous functions. - Fixed regressions where utility functions were truncated and regex for self-closing tags was too greedy. - Ensured 'Module:Tabber' requirement is added whenever a panel is present, including in nested structures. - Improved header rendering to use section titles instead of bolded items where appropriate. - Restored modern JS syntax for consistency and readability. - Verified fix with complex real-world template providing expected tabbed output. Co-authored-by: whostacking <144233917+whostacking@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@script.js`:
- Around line 92-103: readBlocks() currently uses closePattern.exec(...) to find
the first closing tag and thus breaks on nested same-name tags; replace the
first-close regex logic with a depth-aware scan: starting at
tokenPattern.lastIndex iterate through the markup searching for both the same
opening tag (openTag or new RegExp like `<${tagName}[^>]*>`) to increment depth
and the closing tag (`</${tagName}>`) to decrement depth, stop when depth
returns to zero to compute body and full, then push the block and set
tokenPattern.lastIndex to the end of that matched close; update the logic around
the symbols tokenPattern, closePattern/open-tag detection, tagName, openTag,
blocks, and ensure start/body/full are computed from the matching positions
instead of the first regex match.
- Around line 517-519: The luaComments function currently strips HTML markers
but doesn't handle embedded newlines, producing invalid Lua when readComments
preserves newlines; update luaComments to, for each comment (in function
luaComments), remove the leading <!-- and trailing --> as now, then split the
result on /\r?\n/ and map each resulting line to `${indent}-- ${line}`
(flattening the arrays) so every line of a multiline comment is properly
prefixed with the Lua comment marker.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bbc2770-8e2d-4e74-bf67-1b2160575d08
📒 Files selected for processing (2)
index.htmlscript.js
✅ Files skipped from review due to trivial changes (1)
- index.html
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@script.js`:
- Around line 92-103: readBlocks() currently uses closePattern.exec(...) to find
the first closing tag and thus breaks on nested same-name tags; replace the
first-close regex logic with a depth-aware scan: starting at
tokenPattern.lastIndex iterate through the markup searching for both the same
opening tag (openTag or new RegExp like `<${tagName}[^>]*>`) to increment depth
and the closing tag (`</${tagName}>`) to decrement depth, stop when depth
returns to zero to compute body and full, then push the block and set
tokenPattern.lastIndex to the end of that matched close; update the logic around
the symbols tokenPattern, closePattern/open-tag detection, tagName, openTag,
blocks, and ensure start/body/full are computed from the matching positions
instead of the first regex match.
- Around line 517-519: The luaComments function currently strips HTML markers
but doesn't handle embedded newlines, producing invalid Lua when readComments
preserves newlines; update luaComments to, for each comment (in function
luaComments), remove the leading <!-- and trailing --> as now, then split the
result on /\r?\n/ and map each resulting line to `${indent}-- ${line}`
(flattening the arrays) so every line of a multiline comment is properly
prefixed with the Lua comment marker.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bbc2770-8e2d-4e74-bf67-1b2160575d08
📒 Files selected for processing (2)
index.htmlscript.js
✅ Files skipped from review due to trivial changes (1)
- index.html
🛑 Comments failed to post (2)
script.js (2)
92-103:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
readBlocks()still breaks on same-tag nesting.This closes a block at the first matching
</${tagName}>. Nested<panel>,<section>, or<group>containers of the same type will truncate the outer block and shift the remaining scan, so the recursive parser never sees the real tree. This needs a depth-aware scan/stack instead of a first-close regex.🧰 Tools
🪛 OpenGrep (1.22.0)
[ERROR] 94-94: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script.js` around lines 92 - 103, readBlocks() currently uses closePattern.exec(...) to find the first closing tag and thus breaks on nested same-name tags; replace the first-close regex logic with a depth-aware scan: starting at tokenPattern.lastIndex iterate through the markup searching for both the same opening tag (openTag or new RegExp like `<${tagName}[^>]*>`) to increment depth and the closing tag (`</${tagName}>`) to decrement depth, stop when depth returns to zero to compute body and full, then push the block and set tokenPattern.lastIndex to the end of that matched close; update the logic around the symbols tokenPattern, closePattern/open-tag detection, tagName, openTag, blocks, and ensure start/body/full are computed from the matching positions instead of the first regex match.
517-519:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMultiline HTML comments can break the generated Lua.
readComments()preserves embedded newlines, butluaComments()prefixes only the first line with--. A comment like<!-- a\nb -->becomes-- afollowed by rawb, which can syntax-error the emitted module.Suggested fix
function luaComments(comments = [], indent = '') { - return comments.map(comment => `${indent}-- ${comment.replace(/^<!--\s*/, '').replace(/\s*-->$/, '')}`); + return comments.flatMap(comment => + comment + .replace(/^<!--\s*/, '') + .replace(/\s*-->$/, '') + .split(/\r?\n/) + .map(line => `${indent}-- ${line}`) + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script.js` around lines 517 - 519, The luaComments function currently strips HTML markers but doesn't handle embedded newlines, producing invalid Lua when readComments preserves newlines; update luaComments to, for each comment (in function luaComments), remove the leading <!-- and trailing --> as now, then split the result on /\r?\n/ and map each resulting line to `${indent}-- ${line}` (flattening the arrays) so every line of a multiline comment is properly prefixed with the Lua comment marker.
- Refactored `readBlocks()` to handle nested tags of the same name using a depth-aware scan. - Updated `luaComments()` to correctly prefix each line of multiline comments with Lua comment markers. - Added support for `<panel>` and `<section>` tags in Template output, treating them as standard sections and items. - Improved the recursive parser to correctly identify and use child labels/headers as section titles. - Ensured comments are preserved and correctly rendered in both Template and Module outputs. - Fixed a bug where panel sections were not properly categorized in the internal model. - Verified all changes with complex nested structures and real-world template examples. Co-authored-by: whostacking <144233917+whostacking@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
script.js (3)
205-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
<panel>parsing still drops direct non-<section>children.Direct
<data>,<header>,<group>, and<label>children inside<panel>are ignored because onlysub.tagName === 'section'is handled. This can silently remove rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script.js` around lines 205 - 223, The panel parsing currently only collects sub-blocks where sub.tagName === 'section', losing direct children like <data>, <header>, <group>, and <label>; update the loop in the code that builds sections (the block that uses readBlocks(block.body), tabIndex, and pushes to sections) to also handle non-section sub blocks by treating them as rows: for each sub from readBlocks(block.body) call parseBlocks(sub.body, notes) when sub.tagName === 'section' as before but also when sub.tagName is one of 'data','header','group','label' (or any other allowed row types) create a section-less panel row or add those parsed rows into a default section (e.g., a top-level "unnamed" section) so they are not dropped, and ensure the final rows.push({ type: 'panel', sections }) uses that updated sections array; adjust titleRow detection logic (which uses parseBlocks and titleRow = children.find(...)) to work for both explicit sections and the newly collected direct rows.
411-415:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLua title fallback is still resolved in JavaScript, not at Lua runtime.
This bakes a concrete fallback string during conversion (and can bake the converter page title on-wiki), instead of evaluating page title dynamically in generated Lua.
488-491:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNested header rows still bypass
source/defaultfallback resolution.This branch renders only
row.label; headers withsource=and/or<default>inside section/panel paths can collapse to empty bold rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script.js` around lines 488 - 491, The header branch currently inserts raw row.label into infobox (lines using row.type === 'header', tableVar, infobox:renderItem and luaString), which bypasses the source/<default> resolution and can produce empty bold rows; change this branch to run the same source/default resolution used by other row types (the existing field/value resolution codepath your code uses elsewhere) to compute a resolvedLabel (falling back to the <default> or source lookup), then wrap that resolvedLabel with the same bold luaString markup ('''...''') before passing it to infobox:renderItem so headers respect source/default fallback resolution.index.html (1)
67-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose conversion notes as a live region.
notesListis dynamically repopulated, but updates are not announced to screen readers without live-region attributes.Suggested fix
- <ul id="notesList"></ul> + <ul id="notesList" aria-live="polite" aria-atomic="true"></ul>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@index.html` at line 67, The <ul id="notesList"> is dynamically updated but not announced to assistive tech; update the element with live-region attributes (e.g., add aria-live="polite" and aria-atomic="true" on the element with id "notesList") so screen readers are notified of content changes when conversion notes are repopulated; ensure the attributes are present on the element referenced by id "notesList" (or wrap its updates in an element with those attributes) and keep existing update logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@script.js`:
- Around line 147-157: parseBlocks currently pre-pushes every comment into rows
which causes duplicates and misplacement because block rendering later emits
row.comments; remove the pre-pass that iterates "comments" and instead, while
iterating "blocks" (inside parseBlocks), insert comment entries from each
block's "comments" property at the correct position in "rows" (emit { type:
'comment', content: ... } when a block has row.comments) so comments are
produced once and keep their original order; update any code that expects
top-level "comments" to use block.comments and delete the initial loop that
pushes all comments into rows.
- Around line 95-116: The depth scan incorrectly treats close-tags inside HTML
comments as real tags; update the loop that uses
searchPattern/tokenPattern/markup/tagName to detect and skip HTML comments so
they don't affect depth. Concretely, extend the pattern or add logic so matches
that are comment-starts (e.g., begin with "<!--") are consumed/ignored (advance
searchPattern.lastIndex past the comment end "-->") instead of
incrementing/decrementing depth; keep the existing handling for opening tags
(subMatch[1]) and closing tags (subMatch[2]) unchanged. Ensure the check
references the same variables (searchPattern, tokenPattern, markup, tagName,
depth, subMatch) so the scanner continues from after the comment and only counts
real tags.
---
Duplicate comments:
In `@index.html`:
- Line 67: The <ul id="notesList"> is dynamically updated but not announced to
assistive tech; update the element with live-region attributes (e.g., add
aria-live="polite" and aria-atomic="true" on the element with id "notesList") so
screen readers are notified of content changes when conversion notes are
repopulated; ensure the attributes are present on the element referenced by id
"notesList" (or wrap its updates in an element with those attributes) and keep
existing update logic unchanged.
In `@script.js`:
- Around line 205-223: The panel parsing currently only collects sub-blocks
where sub.tagName === 'section', losing direct children like <data>, <header>,
<group>, and <label>; update the loop in the code that builds sections (the
block that uses readBlocks(block.body), tabIndex, and pushes to sections) to
also handle non-section sub blocks by treating them as rows: for each sub from
readBlocks(block.body) call parseBlocks(sub.body, notes) when sub.tagName ===
'section' as before but also when sub.tagName is one of
'data','header','group','label' (or any other allowed row types) create a
section-less panel row or add those parsed rows into a default section (e.g., a
top-level "unnamed" section) so they are not dropped, and ensure the final
rows.push({ type: 'panel', sections }) uses that updated sections array; adjust
titleRow detection logic (which uses parseBlocks and titleRow =
children.find(...)) to work for both explicit sections and the newly collected
direct rows.
- Around line 488-491: The header branch currently inserts raw row.label into
infobox (lines using row.type === 'header', tableVar, infobox:renderItem and
luaString), which bypasses the source/<default> resolution and can produce empty
bold rows; change this branch to run the same source/default resolution used by
other row types (the existing field/value resolution codepath your code uses
elsewhere) to compute a resolvedLabel (falling back to the <default> or source
lookup), then wrap that resolvedLabel with the same bold luaString markup
('''...''') before passing it to infobox:renderItem so headers respect
source/default fallback resolution.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- Fixed `readBlocks` to be depth-aware and correctly skip tags inside HTML comments. - Refactored comment handling in `parseBlocks` to preserve original order and prevent duplicates. - Enhanced `<panel>` parsing to collect all direct children (data, headers, groups) into default sections instead of dropping them. - Improved Lua header rendering to resolve `source` and `<default>` values, consistent with other row types. - Added accessibility live-region attributes to the conversion notes list. - Fixed a bug with greedy regex in `childContent` for self-closing tags. - Ensured consistency in modern JS syntax across `script.js`. Co-authored-by: whostacking <144233917+whostacking@users.noreply.github.com>
This PR adds comprehensive support for tabbed panels () and sections (
PR created automatically by Jules for task 8449397893482632484 started by @whostacking
Summary by CodeRabbit
New Features
Improvements