Skip to content

Conversation

@thatblindgeye
Copy link
Contributor

Closes #668

@patternfly-build
Copy link

patternfly-build commented Nov 12, 2025

Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through Message examples looking for visual regressions manually - looked ok (found unrelated issue #760).

We are experiencing some style bleed from the other components when Markdown is turned on - we likely want to unset or conditionally apply certain classes in the Markdown component mapping when used outside of Messages:

Before After
Screenshot 2025-11-12 at 9 58 08 AM Screenshot 2025-11-12 at 9 57 57 AM
Screenshot 2025-11-12 at 10 00 20 AM Screenshot 2025-11-12 at 10 00 13 AM
Screenshot 2025-11-12 at 10 00 46 AM Screenshot 2025-11-12 at 10 00 39 AM
Before After
Screenshot 2025-11-12 at 10 06 35 AM Screenshot 2025-11-12 at 10 05 53 AM
Screenshot 2025-11-12 at 10 08 17 AM Screenshot 2025-11-12 at 10 06 23 AM
Before After
Screenshot 2025-11-12 at 10 09 46 AM Screenshot 2025-11-12 at 10 09 36 AM
Screenshot 2025-11-12 at 10 10 55 AM Screenshot 2025-11-12 at 10 10 43 AM
Screenshot 2025-11-12 at 10 12 02 AM Screenshot 2025-11-12 at 10 11 54 AM

disallowedElements.push(...reactMarkdownProps.disallowedElements);
}

if (isMarkdownDisabled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you want to pull this out into Message specifically? I don't see us using it in the cards, but it's also not hurting anything. Matter of taste imho.

@thatblindgeye
Copy link
Contributor Author

I think ultimately it may depend how much control the consumer(s) want with this. Right now we're keeping the structures of the 3 components, but allowing Markdown to be passed into various pieces of them. An alternative (or addition) would be to allow consumers to pass completely new Markdown that replaces everything within the CardBody in these components (so no ExpandableSection etc, just the Card and CardBody wrappers with everything else dependent on what the consumers passes as Markdown).

@thatblindgeye thatblindgeye marked this pull request as ready for review November 18, 2025 19:31
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has @kaylachumley reviewed mobile view at all for the cards? Wondering if we need any other adjustments to that or demos for that. I typically test that by placing components in one of the main ChatBot demos - those containers all have breakpoints and styles set up for mobile. I'm concerned about images, etc. in particular.

I'm assuming Kayla has signed off on text styles, so not leaving comments about that. Just a few comments; otherwise looking good so far to me! Thanks for all your hard work on this.

```

### Messages with Markdown in tool response

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edonehoo will probably have feelings here - do we want to put all the tool/thinking stuff with Markdown together? Likely we want to introduce these examples.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, as in - do we want the markdown example to include all of the options in that single block? or pulling together the markdown/non-markdown thinking examples?

My general opinion and first thought is that the current markdown example I think is kind of disruptive to the docs experience since it's so long. Do we need to display every formatting option there? Could we not just state that any message type can be in markdown, show a couple of examples, and let consumers to fill in the gaps for the rest?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @edonehoo here! It does feel lengthy and if possible, it could be nice to show a shorter md examples and just note that any format can be used

/** Whether to strip out images in markdown */
hasNoImages?: boolean;
/** Sets background colors to be appropriate on primary chatbot background */
isPrimary?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tested what this does in the context of the tool calls, etc.? Messages was fine when I looked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed it into markdownContentProps and it did apply pf-m-primary in places and at least on inline code the bakcground color value changed

@kaylachumley kaylachumley self-requested a review November 20, 2025 16:02
@kaylachumley
Copy link

Has @kaylachumley reviewed mobile view at all for the cards? Wondering if we need any other adjustments to that or demos for that. I typically test that by placing components in one of the main ChatBot demos - those containers all have breakpoints and styles set up for mobile. I'm concerned about images, etc. in particular.

I'm assuming Kayla has signed off on text styles, so not leaving comments about that. Just a few comments; otherwise looking good so far to me! Thanks for all your hard work on this.

Good call out! @thatblindgeye Is it possible to see these in a chatbot overlay demo to test how it looks/reacts?

@thatblindgeye
Copy link
Contributor Author

@kaylachumley @rebeccaalpert added a quick demo to show ToolResponse in the overlay chatbot. I added a style to prevent images from causing overflow when in a tool resposne context. ChatBot with markdown tool response

Copy link

@kaylachumley kaylachumley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good - I dont see any issues right now switching from desktop to overlay

Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you - think we just need to iron out the examples with Kayla and Erin (if we want to show examples). Code stuff looks good to me. Peeked at mobile example (thanks for that) and it looked good to me also.

Copy link
Contributor

@nicolethoen nicolethoen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe for a follow up - should we really have a yarn.lock and a package-lock.json file in this project?

}: MarkdownContentProps) => {
let rehypePlugins: PluggableList = [rehypeUnwrapImages, rehypeMoveImagesOutOfParagraphs, rehypeHighlight];
if (openLinkInNewTab) {
rehypePlugins = rehypePlugins.concat([[rehypeExternalLinks, { target: '_blank' }, rehypeSanitize]]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is only appiled when you have 'openLinkInNewTab', does that mean that there are times when we are not sanitizing, when we should?

Copy link
Member

@rebeccaalpert rebeccaalpert Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this because of this callout: https://github.com/rehypejs/rehype-external-links?tab=readme-ov-file#security. I believe the default for the markdown processor we use doesn't need this, but some plugins/components do: https://github.com/remarkjs/react-markdown?tab=readme-ov-file#security

Copy link
Contributor

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a little bit of feedback/a few changes, but in general I think what you wrote already looks great and pretty comprehensive!


To enable Markdown rendering, use the appropriate Markdown flag prop (such as `isBodyMarkdown`, `isSubheadingMarkdown`, or `isExpandableContentMarkdown`) depending on the component and content you're formatting.

**Important:** When using Markdown in these components, set `shouldRetainStyles: true` to retain the styling of the context the Markdown is used in. This ensures that Markdown content maintains the proper font sizes, colors, and other styling properties of its parent component. For example, Markdown passed into a toggle will retain the ChatBot toggle styling, while Markdown in a card body will maintain the appropriate card body styling. Without this prop, the Markdown may override the contextual styles and appear inconsistent with the rest of the ChatBot interface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Important:** When using Markdown in these components, set `shouldRetainStyles: true` to retain the styling of the context the Markdown is used in. This ensures that Markdown content maintains the proper font sizes, colors, and other styling properties of its parent component. For example, Markdown passed into a toggle will retain the ChatBot toggle styling, while Markdown in a card body will maintain the appropriate card body styling. Without this prop, the Markdown may override the contextual styles and appear inconsistent with the rest of the ChatBot interface.
**Important:** When using Markdown in these components, set `shouldRetainStyles: true` to retain the styling of the context the Markdown is used in. This ensures that Markdown content maintains the proper font sizes, colors, and other styling properties of its parent component. For example, Markdown passed into a toggle will retain the ChatBot toggle styling, while Markdown in a card body will maintain the appropriate card body styling. Without this prop, the Markdown may override the contextual styles and create inconsistencies with the rest of the ChatBot interface.


### Tool responses with Markdown

Tool response cards support Markdown in multiple areas including the toggle content, subheading, and body. Use `shouldRetainStyles: true` along with the appropriate Markdown flag props to ensure proper formatting and spacing:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Tool response cards support Markdown in multiple areas including the toggle content, subheading, and body. Use `shouldRetainStyles: true` along with the appropriate Markdown flag props to ensure proper formatting and spacing:
Tool response cards support Markdown in multiple areas, including the toggle content, subheading, and body. Use `shouldRetainStyles: true` along with the appropriate Markdown flag props to ensure proper formatting and spacing.


## Examples with Markdown

The ChatBot supports Markdown formatting in several message components, allowing you to display rich, formatted content. This is particularly useful when you need to include code snippets, lists, emphasis, or other formatted text.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The ChatBot supports Markdown formatting in several message components, allowing you to display rich, formatted content. This is particularly useful when you need to include code snippets, lists, emphasis, or other formatted text.
The ChatBot supports Markdown formatting in several message components, allowing you to display rich, formatted content. This is particularly useful when you need to include code snippets, lists, emphasis, or other formatted text. The following examples demonstrate different ways you can use Markdown in a few of the ChatBot components, but this is not an exhaustive list of all Markdown customizations you can make.

does this sound accurate? if you think your examples cover the majority of styles, we can just leave this off

can consumers theoretically apply any md formatting to any ChatBot components, or are there any limitations we want to address in advance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the addition is good, especially if we tinker more in the future we don't have to wworry about this verbiage being terribly out of date.

I don't think any chatbot components. These ones and Messages are the big ones probably. @rebeccaalpert might have a little more insight

Copy link
Member

@rebeccaalpert rebeccaalpert Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's limited to just these three + messages right now. I think we want to enable more composition in Messages following this work (so people can have Markdown blocks followed by tool calls followed by Markdown blocks, for example), but I'd suggest only enabling Markdown in other components case-by-case as people ask.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay cool, I think this is okay then. But if we want to be more heavy handed in stating that Messages in general can including Markdown, we could always adjust the first sentence to something like "The ChatBot supports Markdown formatting in <Message> components, "

@@ -0,0 +1,265 @@
// ============================================================================
// Markdown Content - Shared component for rendering markdown
// This was aided by Claude code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this line in necessary - it could be in the commit message body rather than in the shipped production code i think.

@nicolethoen nicolethoen merged commit 1663e56 into patternfly:main Dec 3, 2025
7 checks passed
@github-actions
Copy link

github-actions bot commented Dec 3, 2025

🎉 This PR is included in version 6.5.0-prerelease.23 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Lucifergene
Copy link

@thatblindgeye @nicolethoen I'm from the RHOAI team.
We just installed the latest pre-release (6.5.0-prerelease.23) that includes this fix.

But the thing is the copy button for the code-block is broken.
When clicked and pasted, gives me this:

[object Object], ,[object Object],
,[object Object], ,[object Object],
,[object Object],
  ,[object Object], ,[object Object],
  ,[object Object],
    ,[object Object], ,[object Object],
,[object Object],
  ,[object Object], ,[object Object],
  ,[object Object],
    ,[object Object],
      ,[object Object], ,[object Object],
  ,[object Object],
    ,[object Object],
      ,[object Object],
        ,[object Object], ,[object Object],
    ,[object Object],
      ,[object Object],
      ,[object Object], ,[object Object], ,[object Object],
        ,[object Object], ,[object Object],
        ,[object Object],
        ,[object Object], ,[object Object], ,[object Object],
,[object Object],
,[object Object], ,[object Object],
,[object Object], ,[object Object],
,[object Object],
  ,[object Object], ,[object Object],
,[object Object],
  ,[object Object],
    ,[object Object], ,[object Object],
  ,[object Object],
    ,[object Object], ,[object Object], ,[object Object],
      ,[object Object], ,[object Object],
      ,[object Object], ,[object Object],
  ,[object Object], ,[object Object],

It was not an requirement from us, therefore we are totally fine with moving with the previous pre-release that has our requirement.
But wanted to inform this, before anyone finds out!

@thatblindgeye
Copy link
Contributor Author

@Lucifergene just opened #783 to (hopefully) resolve this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool calls/deep thinking/tool response should optionally support markdown input

7 participants