-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix Incomplete Markdown Rendering and Styling Issues in Chat Messages #2632
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
Conversation
…y or another symbolic link (RooCodeInc#2513) * read symbolic linked dir and files recursively * add symlinked dir and nested symlink test case for custom-instructions * enhance comments * add changeset
* changeset version bump * Updating CHANGELOG.md format * Update CHANGELOG.md --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: R00-B0T <[email protected]> Co-authored-by: Matt Rubens <[email protected]>
|
|
Hello @JorkeyLiu, Thank you for your contribution! Upon reviewing the pull request, it seems that the changes are primarily focused on improving markdown rendering and styling in chat messages, which is a cohesive set of changes. However, there are also updates to the content security policy and jest configuration, which might be unrelated to the markdown improvements. Could you please consider splitting the pull request into two separate ones if these changes are not interdependent? For example:
This separation could help in reviewing and testing the changes more effectively. Let me know if you have any questions or if there's anything else I can assist with. Thank you! |
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.
The new MarkdownWithCopy component’s copy button currently appears only on hover. Consider adding keyboard accessibility (e.g. via tabindex and onFocus events) to ensure users on touch devices or keyboard-only users can also trigger the copy action.
|
Any chance we can add the a11y suggestion from ellipsis-bot for keyboard accessibility? Thanks |
If we were to split the CSP and Jest changes into a separate PR, the Markdown improvements in this PR would effectively be broken upon merging, and the related unit tests would fail, until the other changes were also merged. |
b5072c0 to
d1d9c84
Compare
|
This commit will also fix chat view flickering issue (BTW) |
|
@JorkeyLiu Do you have discord account ? |
|
Thank you for standardizing the markdown display! FYI: CodeBlock was just merged with Shiki rendering to handle all multi-line syntax-highlighting, and it is used consistently in many places throughout the code base. Feel free to style Shiki consistently since it was merged as a stable feature in 3.15. Shiki supports many existing themes, currently only I see that you have a copy button on your codeblock sample above, so in the interest of avoiding duplicate work it would be great to continue using the existing CodeBlock Shiki implementation;
|
|
hope this pr to be merged, to solve the markdown 'unorder list' and 'order list' render problem |
|
Thank you for the PR. I’ll review closely today. |
|
I attempted to merge main into this branch - happy to push if it's not stepping on toes. The table and ordered/unordered list rendering is a great improvement! I did notice some issues though:
Is there a way for us to port the tables and lists to the markdown component that we're currently using while we try to align on the path forward, or will that be a lot of wasted work? |
I am not a fan of the However: I really like that the author is standardizing markdown CSS, fixing bullets, headings, enumerations, etc. If we can keep the new markdown CSS for everything else, and have still render multi-line code blocks using the existing Proposed solution:
Additionally, The following needs to be tested in this Markdown rework before merging:
@samhvw8 can you think of anything else that would need to be tested in markdown user interface if the markdown rendering engine changes? |
|
Thanks for your reviewing.
The code blocks should be rendered after streaming output, but there may have some timing issues sometimes, I'll make time to look into it.
Currently yes
The bottom button is for copying the whole markdown content, it does the same as always.
It cannot be simply ported or migrated to table({ children }) {
return <table className="w-full my-2 border-collapse border border-border">{children}</table>
},That's why I post this pr, it seems a mistake to use
I appreciate @KJ7LNW 's work very much. I almost migrated the navigation feature to |
Please retain the current real-time stream-rendering, as it does now, because it is quite useful for review of very long commands (eg, Really everything needs to support real-time stream-rendering.
There is a copy-as-markdown feature presently, so definitely keep that; I use it all the time and others may too.
Please do, lets avoid any regressions. |
|
Please convert this PR to a draft so it does not get merged before we are done with review. |
122b9fe to
799cf15
Compare
|
Fixed code blocks rendering issue, now it will be rendered 100% after streaming output. |
|
I see that you imported Shiki, but this is what we need to do for this first pull request:
|
@mrubens I think this makes sense. |
|
stale |

Context
The Markdown rendering previously used for chat messages (primarily via
ChatRow.tsx) was incomplete and failed to render several standard Markdown elements correctly. Notably, list markers (both unordered bullets and ordered numbers) were not displayed, and GFM elements like tables appeared as unformatted plain text.After switching to a more standard-compliant Markdown renderer (
@/components/ui/markdown/Markdown.tsx) to address these fundamental rendering issues, further visual inconsistencies were observed compared to the previous implementation. These included differences in list indentation depth, bold text weight, and the styling (background, font, padding, border, alignment) of inline code blocks.The primary root cause was the use of an incomplete and non-standard Markdown renderer (
MarkdownBlock.tsx) in the main chat message rendering path. Secondary styling issues arose from the inherent visual differences between the oldstyled-components-based styling and the new Tailwind CSS-based styling after switching to the standard renderer.Implementation
@/components/common/MarkdownBlock.tsx: Usedreact-remarkandstyled-components. This implementation lacked complete CommonMark support (missing list markers) and GFM support (no tables). It had custom styling, auto-URL linking, and Mermaid support.@/components/ui/markdown/Markdown.tsx: Usesreact-markdownwithremark-gfmand Tailwind CSS. Provides more robust and standard-compliant Markdown rendering, including GFM features.ChatRow.tsx, responsible for rendering many chat messages, was incorrectly relying on the limitedMarkdownBlock.tsximplementation.ChatRow.tsxto consistently use the shared, standard-compliant@/components/ui/markdown/Markdown.tsxcomponent. This resolved the fundamental rendering issues for lists, tables, and other standard elements.@/components/ui/markdown/Markdown.tsxcomponent to address visual discrepancies introduced by the renderer switch. Adjustments included list indentation (pl-[2.5em]), bold weight (font-semibold), inline code styling (background, color, padding, border, font, alignment), and ensuring the base font size usesvar(--vscode-font-size).ChatRow.tsx's local wrapper) by creating a new wrapper component (MarkdownWithCopy) withinChatRow.tsxaround the shared<Markdown />component. This restored specific UI interactions and layout nuances desired for chat messages.The solution involved replacing the deficient renderer with a standard-compliant one (
react-markdown+remark-gfm), unifying the Markdown implementation used byChatRow.tsx. Subsequently, styles were adjusted in the shared component, and specific UI features/layout wrappers were reintroduced in the calling component (ChatRow.tsx) to achieve functional and visual consistency with the intended design.Screenshots
How to Test
To Reproduce
Steps to reproduce the behavior:
ChatRow.tsx).Expected behavior
font-semibold).--vscode-textCodeBlock-background), appropriate padding, border, font, and alignment consistent with the surrounding text.Get in Touch
[email protected]
Important
Refactored chat message rendering to use a standard-compliant Markdown renderer, fixed styling issues, and restored hover-to-copy functionality.
ChatRow.tsxto useMarkdown.tsxfor standard-compliant Markdown rendering, fixing issues with lists, tables, and other elements.ChatRow.tsxusing a newMarkdownWithCopywrapper.Markdown.tsxfor list indentation, bold text weight, and inline code styling.CodeBlock.tsxto handle code highlighting based on completion status and theme.react-markdown,remark-gfm, andshikiinjest.config.cjsfor testing purposes.package.jsonto includeshikias a dependency.This description was created by
for 356831df0f38639fda4c1d0aa09121e6ad5e8771. It will automatically update as commits are pushed.