-
Notifications
You must be signed in to change notification settings - Fork 0
MarkdownViewer article and demos #696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
hyyan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurenic0l This article has a few issues. The flow is not completely smooth, and it makes some incorrect assumptions.
-
Conflates "streaming" with "progressive rendering"
append()works independently - content appears immediately if progressiveRender is off- "Streaming methods" section title is misleading
- Streaming is about receiving data from server
- Progressive rendering is about displaying it character-by-character
-
Missing dependency clarity
isRendering()only meaningful when progressiveRender is enabledstop(),flush(),whenRenderComplete()only work with progressiveRender on- These dependencies aren't explained
-
Auto-scroll placement
- Presented as if related to streaming, but it's completely independent
- Can be used with or without streaming/progressive rendering
-
Organization implies wrong flow
- Suggests you need "streaming methods" for progressive rendering
- Reality: you can append() without progressiveRender
I suggest this structure, please verify:
- Setting content
- Appending content (for any incremental updates)
- Auto-scroll (independent feature)
- Progressive rendering (visual typewriter effect)
- Enable it
- Render speed
- Render state (isRendering)
- Controlling rendering (stop/flush)
- Waiting for completion (whenRenderComplete)
- Clearing content
- Syntax highlighting
src/main/java/com/webforj/samples/views/markdownviewer/MarkdownViewerView.java
Outdated
Show resolved
Hide resolved
src/main/java/com/webforj/samples/views/markdownviewer/MarkdownViewerView.java
Show resolved
Hide resolved
src/main/java/com/webforj/samples/views/markdownviewer/MarkdownViewerStreamingView.java
Outdated
Show resolved
Hide resolved
src/main/java/com/webforj/samples/views/markdownviewer/MarkdownViewerStreamingView.java
Show resolved
Hide resolved
62c2859 to
fde2508
Compare
| MarkdownViewer viewer = new MarkdownViewer("# Hello World"); | ||
|
|
||
| // Replace content entirely | ||
| viewer.setContent("## New Content\n\n- Item 1\n- Item 2"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use java multiline strings instead for readability
| card.setStyle("maxWidth", "600px") | ||
| .setStyle("width", "100%") | ||
| .setStyle("padding", "var(--dwc-space-l)") | ||
| .setStyle("background", "var(--dwc-color-surface-1)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that should be: var(--dwc-surface-3)
| viewer.append("More content here..."); | ||
| ``` | ||
|
|
||
| By default, appended content appears immediately. When progressive rendering is enabled, appended content goes into a buffer and displays character-by-character instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link progressive rendering section
| viewer.setRenderSpeed(speed); | ||
| }); | ||
|
|
||
| viewer.setProgressiveRender(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add var(--dwc-surface-3) background please
| .setSpacing("var(--dwc-space-m)") | ||
| .setPadding("var(--dwc-space-l)"); | ||
|
|
||
| speedChoice.setLabel("Render Speed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align to right and give it 200px width please
| return italicText; | ||
| } | ||
|
|
||
| public Locator getStrikethrough() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not used anywhere
| import com.webforj.samples.pages.markdownviewer.MarkdownViewerProgressivePage; | ||
| import com.webforj.samples.views.BaseTest; | ||
|
|
||
| public class MarkdownViewerProgressiveViewIT extends BaseTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests only check element visibility on initial load - they don't verify any behavior or content. For example, the streaming test never types a message, clicks send, or checks that streaming starts. The basic viewer test checks that a tag exists somewhere, but not that the markdown rendered the expected text. These tests are too shallow to catch regressions - they'll keep passing even when functionality is broken. we should be testing actual behavior (send a message, verify response appears) and asserting on expected content rather than just element presence.
| messagesArea.addClassName("chat__messages"); | ||
| messagesArea.setStyle("overflowY", "auto"); | ||
|
|
||
| viewer.setProgressiveRender(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable auto scroll
| <p>A component for displaying user profile pictures or initials, with support for different sizes, shapes, and themes.</p> | ||
| </GalleryCard> | ||
|
|
||
| <GalleryCard header="MarkdownViewer" href="markdownviewer" image="/img/components/MarkdownViewer.png"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update the screenshot after you fix the demo background to use Surface 3.
|
|
||
| ## Syntax highlighting {#syntax-highlighting} | ||
|
|
||
| The `MarkdownViewer` supports syntax highlighting for code blocks when Prism.js is available. Add Prism.js to your app using the `@JavaScript` and `@StyleSheet` annotations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's link Prism.js
|
@MatthewHawkins @laurenic0l, we'll need to get this fixed before we can merge. |
Here's the first draft of the MarkdownViewer documentation. POM file should be updated with 25.11
closes #659