Skip to content

fix(assistant): render markdown in TextInput output#540

Open
n-iv wants to merge 2 commits into
nextcloud:mainfrom
n-iv:fix/render-markdown-in-assistant-output
Open

fix(assistant): render markdown in TextInput output#540
n-iv wants to merge 2 commits into
nextcloud:mainfrom
n-iv:fix/render-markdown-in-assistant-output

Conversation

@n-iv
Copy link
Copy Markdown

@n-iv n-iv commented May 13, 2026

Summary

The assistant's "generate text" form currently displays its output through NcRichContenteditable, which renders text as-is. When the underlying LLM returns markdown (heading, lists, tables, bold, links), the user sees raw markdown syntax instead of formatted content.

This PR wraps the output field in NcRichText when isOutput is true, falling back to NcRichContenteditable for the input case. Headings, lists, bold, italic, code blocks and links are now rendered. Tables are not rendered due to the markdown-it configuration of NcRichText in @nextcloud/vue, which does not enable the table plugin by default. The copy still functions and keeps the markdown as expected.

Screenshots

Before
Screenshot_20260513_102348

After
Screenshot_20260513_130308

Wrap the output field in NcRichText when isOutput is true, falling back to NcRichContenteditable for the input case. Headings, lists, bold, italic, code blocks and links are now rendered. Tables are not rendered due to the markdown-it configuration of NcRichText in @nextcloud/vue, which does not enable the table plugin by default. The copy still functions and keeps the markdown as expected.

Signed-off-by: niv <nicolas.varlot@ac-versailles.fr>
Copilot AI review requested due to automatic review settings May 13, 2026 12:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Renders markdown content in the assistant's "generate text" output by using NcRichText for output mode while keeping NcRichContenteditable for input mode.

Changes:

  • Conditionally render NcRichText when isOutput && hasValue is true.
  • Import and register the NcRichText component.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@janepie
Copy link
Copy Markdown
Member

janepie commented May 15, 2026

Thank you for this PR! The problem with this is that is makes the output uneditable for the user, and we assume that the LLM output is not perfect and has to be adapted. Can you find a way to keep that function, and also keep the "easy copy" button?

@n-iv
Copy link
Copy Markdown
Author

n-iv commented May 18, 2026

Hi @janepie, the easy copy button is still present and can be used. I added a very short video to show it.
Screencast_20260518_101227.webm

@janepie
Copy link
Copy Markdown
Member

janepie commented May 19, 2026

Ah thanks for showing me! It's a bit unfortunate that the button and the AI-generation note disappear into the scrollable area, we should keep them visible and make the result scrollable instead.

So points to adapt would be

  • keep an easy way to edit the result (for example change into edit mode with a doubleclick on the area)

  • always keep copy button and AI note in sight and make the result scrollable instead

Wrap the rendered output in a bounded scrollable container and allow
  switching to edit mode with a double-click. The contenteditable returns
  to rendered mode on blur. This preserves the editing capability while
  keeping the markdown rendering as the default view.
  
  Addresses review feedback on nextcloud#540.

Signed-off-by: Nicolas Varlot <nicolas.varlot@ac-versailles.fr>
@n-iv
Copy link
Copy Markdown
Author

n-iv commented May 20, 2026

I pushed a new commit addressing both points:

  • Double-clicking the rendered output now switches it to an editable contenteditable, which returns to rendered mode on blur. A title attribute hints at the affordance.
  • The rendered output (and the editable variant in output mode) is now wrapped in a bounded scrollable container, so the result scrolls inside the component instead of pushing the copy button and the AI-generation note out of sight.
Screenshot_20260520_121515 Screenshot_20260520_121648 Screenshot_20260520_121825 Screenshot_20260520_121838

Copy link
Copy Markdown
Member

@janepie janepie left a comment

Choose a reason for hiding this comment

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

Looks good!
I'd prefer to have the double-click hint somewhere more visible but I'm not sure yet where to put it, so let's do that in a follow-up.

A few small remarks

<br v-if="limitLabel">
{{ limitLabel ?? '' }}
</label>
<div
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why wrapping it in a div? Should work with directly using NcRichText

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the wrapper can be easily removed, I agree with Jana it would be nice to remove it. But if it's way more convenient to have it because styling NcRichText directly is sketchy, then let's keep the wrapper.

.copy-button,
.choose-file-button {
position: absolute !important;
z-index: 10;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's this for?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tried removing it and couldn't see any difference. Is it fixing something in a very special case?

@janepie janepie requested a review from julien-nc May 21, 2026 09:41
Copy link
Copy Markdown
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Nice change! I like the increase of the max-height.

To make this more discoverable, there could be an "edit" button in the top right corner when it's not editable. Wdyt @janepie and @n-iv ? It would require to slightly increase the min-height so this edit button does not overlap with the copy one.

I spotted a minor bug, when the field is empty, we can only type one character and it leaves the edit mode. A double click is necessary to continue editing. This does not happen when the field is not empty. I'd say it's fine if we don't find an easy fix for that.

.copy-button,
.choose-file-button {
position: absolute !important;
z-index: 10;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tried removing it and couldn't see any difference. Is it fixing something in a very special case?

display: block !important;
box-sizing: border-box !important;
border: 2px solid var(--color-primary-element) !important;
border-radius: var(--border-radius) !important;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be --border-radius-large to be consistent with the NcRichContenteditable border radius.

border: 2px solid var(--color-primary-element) !important;
padding-bottom: 38px !important;
max-height: 35vh !important;
overflow-y: auto !important;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.rich-contenteditable__input already has overflow-y: auto set by the server style. This can be removed.

<br v-if="limitLabel">
{{ limitLabel ?? '' }}
</label>
<div
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the wrapper can be easily removed, I agree with Jana it would be nice to remove it. But if it's way more convenient to have it because styling NcRichText directly is sketchy, then let's keep the wrapper.

padding-bottom: 42px !important;
max-height: 35vh !important;
overflow-y: auto !important;
cursor: text;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not enough.

Suggested change
cursor: text;
.rendered-output, .rendered-output * {
cursor: text;
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants