Skip to content
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

MSC3765: Rich text in room topics #3765

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

Johennes
Copy link
Contributor

@Johennes Johennes commented Apr 3, 2022

Rendered

Implementations:


In line with matrix-org/matrix-spec#1700, the following disclosure applies:

I am a Systems Architect at gematik, Software Engineer at Unomed, Matrix community member and former Element employee. This proposal was written and published with my community member hat on.


FCP tickyboxes

Signed-off-by: Johannes Marbach <johannesm@element.io>
@Johennes Johennes changed the title MSC3677: Rich text in room topics MSC3765: Rich text in room topics Apr 3, 2022
@turt2live turt2live added proposal-in-review proposal A matrix spec change proposal client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff labels Apr 3, 2022
@alphapapa

This comment was marked as duplicate.

@Johennes

This comment was marked as duplicate.

@emorrp1

This comment was marked as duplicate.

@@ -0,0 +1,96 @@
# MSC3765: Rich text in room topics
Copy link
Member

Choose a reason for hiding this comment

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

@alphapapa says:

On one hand, I can see some elegance in repurposing room topics for general-purpose, long-term room reference information. OTOH, it seems like overloading the purpose of topics with what, in other systems, would go in "pinned" topics or messages, or a wiki, etc.

So IMHO I would consider implementing support for pinned messages/events before overloading topics like this. It would seem relatively straightforward for a room's state to have a list of pinned events, which could be sent in initial sync by the server or be retrieved manually by clients. Clients could then display these pinned events in a room's timeline view, optionally hiding them, compressing them, etc. And the pinned events could be edited by room moderators using existing event-editing tools. (Forgive me if there's already a proposal for something like that.)

Copy link
Member

Choose a reason for hiding this comment

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

@Johennes replies:

Interesting idea. Pinned events seem to already exist. However, in their current form, these are not fit to be used for what you describe because, depending on room settings, users joining the room after the events were sent could be unable to see them.

@turt2live
Copy link
Member

@alphapapa and others: please use threads on the diff to have your comments considered. This can be done by adding a line comment.

If there's no obvious line for where to put a comment, please use the line containing the title.

@Johennes
Copy link
Contributor Author

Johennes commented Feb 2, 2024

The unresolved comments need to be addressed and afterwards I think it could be sent through FCP which might yield further comments.

Update (2024-02-23): I've resolved the remaining comments and raised the proposal with the SCT.

@famfop
Copy link

famfop commented Aug 26, 2024

Hi, what is the state about this PR? AFAIK, Markdown in Room topics (my use-case) is not yet supported. This conversation looks like everything looks good, is there anything stopping it from getting merged?

@tmonz
Copy link

tmonz commented Aug 26, 2024

it has been added to development/nightly, and seems to be working there flawlessly for a long time - from my own testing

proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Overall the MSC looks pretty good. In my opinion, once the backwards compatibility (old room versions) thread is resolved, this should be ready for FCP. Let us know in the SCT Office once that's been done, as we might miss the github notification (sorry).

proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
@turt2live
Copy link
Member

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Oct 22, 2024

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

  • binds behaviour of future unspecified versions

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Oct 22, 2024
Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

Not a blocking concern but it would be great to polish the wording a little.

proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
proposals/3765-rich-room-topics.md Outdated Show resolved Hide resolved
@tulir tulir mentioned this pull request Oct 26, 2024
Co-authored-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Co-authored-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Comment on lines +16 to +20
Drawing from extensible events as described in [MSC1767], `m.room.topic`
is prohibited in room versions that support extensible events and replaced
with a new `m.topic` event type. The latter contains a new content block
`m.topic` which wraps an `m.text` content block that allows representing
the room topic in different mime types.
Copy link
Member

Choose a reason for hiding this comment

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

I read this and thought "well, since there are no current room versions that support extensible events, we're not going to be able to use this anywhere".

Having read through a bit more and come across the "Transition" and "Notes" sections, I realise that's not really the case.

What's more: I am worried that any behaviour defined here for some mythical extensible-events-supporting room version is going to be forgotten by the time we actually come to define such a room version; and then we will find ourselves very confused about what is or is not specced. I would strongly argue that we should limit this proposal to the room versions we have today, with some non-normative suggestions about how how things might look in future room versions.

In other words: let's rephrase this a bit.

Suggested change
Drawing from extensible events as described in [MSC1767], `m.room.topic`
is prohibited in room versions that support extensible events and replaced
with a new `m.topic` event type. The latter contains a new content block
`m.topic` which wraps an `m.text` content block that allows representing
the room topic in different mime types.
Drawing from extensible events as described in [MSC1767], a new content block
`m.topic` is defined, which wraps an `m.text` content block that allows representing
the room topic in different mime types.
In current room versions, this content block is added to the content of
[`m.room.topic`](https://spec.matrix.org/v1.12/client-server-api/#mroomtopic) events as shown below.
See also (link to separate section) which discusses possible adoption in future room versions which
support extensible events.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively: we don't have to wait for the whole of extensible events to land to make m.topic events a thing: it's not even clear we need a new room version. (Room versions have historically focussed on server-side functionality, whereas topics are more a client-side thing. Clients generally accept unknown room versions and muddle through as best they can.)

Anyway, the point here is that there are options here. Let's just not define behaviour for a room version so far away that we'll forget about this in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sounds like this would mean rebasing this proposal onto the current spec (which doesn't know extensible events) and then creating another proposal to adapt it to the extensible events room version later?

I'd be ok with that but I feel like I'm being bounced back and forth a little here. @turt2live this would probably benefit from your input.

@richvdh I guess you might want to stick up a concern?

Comment on lines +22 to +38
``` json5
{
"type": "m.topic",
"state_key": "",
"content": {
"m.topic": {
"m.text": [{
"body": "All about **pizza** | [Recipes](https://recipes.pizza.net)"
}, {
"mimetype": "text/html",
"body": "All about <b>pizza</b> | <a href=\"https://recipes.pizza.net\">Recipes</a>"
}]
}
},
...
}
```
Copy link
Member

Choose a reason for hiding this comment

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

replace with example of m.room.topic event

@richvdh
Copy link
Member

richvdh commented Nov 6, 2024

@mscbot concern binds behaviour of future unspecified versions

@mscbot mscbot added the unresolved-concerns This proposal has at least one outstanding concern label Nov 6, 2024
Co-authored-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API disposition-merge kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. unresolved-concerns This proposal has at least one outstanding concern
Projects
Status: Ready for FCP ticks
Development

Successfully merging this pull request may close these issues.