fix: resolve interactive-book rendering issue#519
fix: resolve interactive-book rendering issue#519yashmanjunath-74 wants to merge 1 commit intoCircuitVerse:masterfrom
Conversation
✅ Deploy Preview for cv-mobile-app-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis pull request modifies the interactive book rendering pipeline across multiple syntax parsers and builders. Changes include: extracting content from element attributes instead of text content, wrapping various elements (iframes, interactions, quiz content) in paragraph containers, replacing null returns with placeholder blocks for stack safety, and introducing an empty builder that renders nothing. These modifications adjust how markdown elements are parsed and structured before rendering. Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/ui/views/ib/syntaxes/ib_liquid_syntax.dart (1)
19-31:⚠️ Potential issue | 🟠 MajorGuard malformed include tags to prevent parser crashes.
The image and interaction branches still assume required tokens/attributes exist. Missing
url,description, or a second tag token can throw and break page rendering.Proposed defensive parsing fix
- } else if (tags[1] == 'image.html' && tags.length >= 3) { + } else if (tags.length > 1 && tags[1] == 'image.html') { // Images - var url = - RegExp(r'''url=("|')([^"'\n\r]+)("|')''').firstMatch(match[1]!)![2]; - var alt = - RegExp( - r'''description=("|')([^"'\n\r]*)("|')''', - ).firstMatch(match[1]!)![2]; - - var img = md.Element.withTag('img'); - img.attributes['src'] = '${EnvironmentConfig.IB_BASE_URL}$url'; - img.attributes['alt'] = alt!; - node = md.Element('p', [img]); + final urlMatch = + RegExp(r'''url=("|')([^"'\n\r]+)("|')''').firstMatch(match[1]!); + final altMatch = RegExp( + r'''description=("|')([^"'\n\r]*)("|')''', + ).firstMatch(match[1]!); + + final url = urlMatch?.group(2); + final alt = altMatch?.group(2) ?? ''; + if (url == null || url.isEmpty) { + node = md.Element('p', [md.Element.withTag('empty')]); + } else { + final img = md.Element.withTag('img'); + img.attributes['src'] = '${EnvironmentConfig.IB_BASE_URL}$url'; + img.attributes['alt'] = alt; + node = md.Element('p', [img]); + } } else { // Interactions using html // Use attribute 'id' for data-via-attributes fix - var intNode = md.Element.withTag('interaction'); - intNode.attributes['id'] = tags[1]; - node = md.Element('p', [intNode]); + if (tags.length > 1) { + final intNode = md.Element.withTag('interaction'); + intNode.attributes['id'] = tags[1]; + node = md.Element('p', [intNode]); + } else { + node = md.Element('p', [md.Element.withTag('empty')]); + } }Also applies to: 34-37
lib/ui/views/ib/builders/ib_interaction_builder.dart (1)
17-21:⚠️ Potential issue | 🟠 MajorShort-circuit when interaction
idis empty.When
idis missing, the builder still issuesfetchHtmlInteraction(''), which can trigger avoidable failed requests and repeated loading/error UI.Proposed fix
- var id = element.attributes['id'] ?? ''; + final id = (element.attributes['id'] ?? '').trim(); + if (id.isEmpty) { + return const SizedBox.shrink(); + } return FutureBuilder<dynamic>( future: model.fetchHtmlInteraction(id),
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/ui/views/ib/builders/ib_interaction_builder.dartlib/ui/views/ib/builders/ib_pop_quiz_builder.dartlib/ui/views/ib/ib_page_view.dartlib/ui/views/ib/syntaxes/ib_embed_syntax.dartlib/ui/views/ib/syntaxes/ib_filter_syntax.dartlib/ui/views/ib/syntaxes/ib_liquid_syntax.dartlib/ui/views/ib/syntaxes/ib_md_tag_syntax.dart
2 participants
Fixes #496
Describe the changes you have made in this PR -
Summary
This PR addresses a critical assertion failure (_inlines.isEmpty) in the flutter_markdown builder that occurred during interactive scrolling in the Interactive Book section. The fix implements a robust, stack-safe architecture to ensure the markdown parser and builder remain perfectly synchronized.
The Problem
The diagnostic logs revealed that the _inlines.isEmpty error was caused by a fundamental leak in the markdown builder's inline stack. This happened when:
BlockSyntax nodes returned null or "Empty Text" children after advancing the parser.
Custom tags were interpreted as inlines instead of blocks, preventing the builder from clearing the inline stack correctly during its traversal.
The Solution: 100% Stack-Safe Architecture
Parser Synchronization
Updated BlockSyntax implementations (
IbMdTagSyntax,
IbLiquidSyntax,
IbFilterSyntax
) to never return null after advancing the parser. If content is skipped, the parser now returns an empty block node wrapped in a
tag. This prevents the parser from getting stuck or producing unbalanced nodes.
Paragraph Protection
Forced the MarkdownBuilder to follow its most stable and well-tested stack-clearing pathway by wrapping all custom block elements (like chapter_contents, interaction, and iframe) in native Paragraph (p) elements. Since p is a built-in block tag, it ensures the inline stack is cleared before proceeding to the next node.
Data-via-Attributes Refactoring
Refactored builders to read their IDs and content from attributes instead of text content. This allows the syntaxes to return nodes with strictly empty children lists, which prevents the builder from ever pushing a "leaky" entry onto the inline stack.
Interactions: Now use attributes['id'].
Pop Quizzes: Now use attributes['content'].
4. Invisible Empty Placeholder
Introduced a dedicated
IbEmptyBuilder
that renders SizedBox.shrink() for placeholder nodes (). This allows the parser to remain synchronized without impacting the UI layout.
Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit