Skip to content

feat(chat-message): remove contentFormatter in favor of content projection - #2584

Open
dr-itz wants to merge 6 commits into
mainfrom
feat/chat/remove-contentFormatter
Open

feat(chat-message): remove contentFormatter in favor of content projection#2584
dr-itz wants to merge 6 commits into
mainfrom
feat/chat/remove-contentFormatter

Conversation

@dr-itz

@dr-itz dr-itz commented Aug 18, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

@dr-itz dr-itz changed the title Feat/chat/remove content formatter feat(chat-message): remove contentFormatter in favor of content projection Aug 18, 2026
@dr-itz

dr-itz commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@Killusions @robertwilde @spliffone WDYT?

@dr-itz
dr-itz force-pushed the feat/chat/remove-contentFormatter branch from 4ea2456 to e210c14 Compare August 18, 2026 20:47
@Killusions

Killusions commented Aug 19, 2026

Copy link
Copy Markdown
Member

Makes sense with the new Markdown renderer.

spliffone
spliffone reviewed Aug 19, 2026
View reviewed changes

@spliffone spliffone left a comment

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 love it, the changes make totally sense and improve the security.

Comment thread projects/element-ng/schematics/ng-update/index.spec.ts Outdated
Comment thread projects/element-ng/schematics/ng-update/index.spec.ts Outdated
Comment thread projects/element-ng/schematics/ng-update/migrate-content-formatter.ts
@dr-itz
dr-itz marked this pull request as ready for review August 19, 2026 14:53
@dr-itz
dr-itz requested review from a team as code owners August 19, 2026 14:53
@dr-itz
dr-itz force-pushed the feat/chat/remove-contentFormatter branch 2 times, most recently from 5eaeeb3 to f4fa912 Compare August 22, 2026 00:59
@dr-itz dr-itz added the breaking-changes Marks issues and PRs that are breaking the API label Aug 23, 2026
@dr-itz

dr-itz commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

And the follow-up will be removing si-markdown-renderer. Or should I fold into this PR?

@spike-rabbit

spike-rabbit commented Aug 24, 2026

Copy link
Copy Markdown
Member

@spliffone any more comments on this one, or can I proceed and merge?

@dr-itz I think since this is ready, lets remove it in a follow-up.

spliffone
spliffone reviewed Aug 24, 2026
View reviewed changes
Comment thread projects/element-ng/chat-messages/si-ai-message.component.spec.ts
spliffone
spliffone reviewed Aug 24, 2026
View reviewed changes
Comment thread projects/element-ng/chat-messages/si-user-message.component.spec.ts
@dr-itz
dr-itz force-pushed the feat/chat/remove-contentFormatter branch from f4fa912 to 5ec38fc Compare August 25, 2026 12:15
@dr-itz

dr-itz commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

since there were two comments to address regarding testing, I folded the removal of si-markdown-renderer into this one. with schematics.

@dr-itz
dr-itz force-pushed the feat/chat/remove-contentFormatter branch from 5ec38fc to 70156ba Compare August 25, 2026 13:43
@kfenner

kfenner commented Aug 26, 2026

Copy link
Copy Markdown
Member

@dr-itz This is awesome, I don't have anything to comment regarding the code changes.

However, I think non of these commits should be feat. We already have the BREAKING CHANGE notes, we don't need to see the same things under "Features". And for schematics, do we really announce these as features? I'd expect that's more some chore thing, no?

/cc @spike-rabbit WDYT?

@spike-rabbit

spike-rabbit commented Aug 27, 2026

Copy link
Copy Markdown
Member

For the schematics we usually have them listed as feat or in the breaking change note. Depending on when they got added.
In this case I believe just having them in the mentioned in the breaking note should be fine.
Barely anyone should be affected by this anyway.

dr-itz added 6 commits August 27, 2026 12:25
@dr-itz
chore(chat-message): remove contentFormatter in favor of content proj…
b039e65 
…ection

BREAKING CHANGE: For `si-ai-message` and `si-user-message`, the
`contentFormatter` input is no longer available. Instead use content projection.
For markdown content, use the new `si-markdown` component. There's a schematic
to automatically upgrade.
@dr-itz
chore(schematics): add migration from contentFormatter to si-markdown
0f7750c
@dr-itz
chore(schematics): remove unused getMarkdownRenderer and DomSanitizer
7d66153
@dr-itz
test(chat-message): add tests for content projection and text content
61fb632
@dr-itz
chore(schematics): add migration from si-markdown-renderer to si-mark…
39cc831 
…down
@dr-itz
chore(markdown-renderer): remove si-markdown-renderer and getMarkdown…
4bf4a4f 
…Renderer

BREAKING CHANGE: si-markdown-renderer and getMarkdownRenderer have been
removed in favor of the new si-markdown component. There's a schematic
to replace the component.
@dr-itz
dr-itz force-pushed the feat/chat/remove-contentFormatter branch from 70156ba to 4bf4a4f Compare August 27, 2026 10:26
@dr-itz

dr-itz commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

@spike-rabbit @kfenner ok, feat are gone

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

Reviewers

@spliffone spliffone spliffone left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

breaking-changes Marks issues and PRs that are breaking the API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dr-itz @Killusions @spike-rabbit @kfenner @spliffone