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

MSC2875: room descriptions #2875

Closed
wants to merge 6 commits into from
Closed

MSC2875: room descriptions #2875

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 23, 2020

Rendered

Signed-off-by: mewmew alexafediverse@gmail.com

@uhoreg uhoreg added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels Nov 23, 2020
Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

A bunch of nit-picks

proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
proposals/2875-roomdescriptions.md Show resolved Hide resolved
proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
While this msc is in development, the event should be sent as `cat.blob.msc2875.description` instead of `m.room.description`.

# Alternatives
`m.room.pinned_events` can be used, though not all clients may have access to pinned events, causing issues in many rooms, as well as less consistent display, usage, and support.
Copy link
Member

Choose a reason for hiding this comment

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

A new event type is probably not a good solution for inconsistent client support

Copy link
Member

Choose a reason for hiding this comment

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

I agree; lack of adoption of something already specced is a rather poor argument for speccing yet another event type. The relevant argumentation against using m.room.pinned_events, however, would probably be the intentionally longer form of the description text, compared to that in message events. The UI to enter messages in most prominent clients is not exactly facilitating that longer form.

Copy link
Author

Choose a reason for hiding this comment

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

This is more of a "pinned messages aren't intended to be used for a long-form description".

Depending on the room config, and federation weirdness, the messages might not be available to most users who join the room. A specific event is a much better solution here, so clients can consistently display a description instead of a list of pinned messages.

proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
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.

I have no certain opinion about this MSC yet but here are a few comments on the current text.

HTML elements can also inlude the `id` property and links to fragments (such as `<h2 id="example">Example</h2>` and `<a href="#example">jump to example</a>` for easy navigation or table of contents.

# Client implementation
Clients should either expose editing the description as raw HTML, or as Markdown, though translation to markdown may be lossy. This is left up to client developers.
Copy link
Member

Choose a reason for hiding this comment

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

Editing raw HTML for anything beyond smaller snippets is not exactly good experience for non-technical audience; basically, having HTML here mandates a rich text editor, internal or external. Markdown is slightly better but, yes, HTML-to-Markdown conversion is lossy.

Overall, it seems to me that a bit more investigation into pros and contras of possible choices for the text format here really wouldn't hurt. The point is not to argue about what's "right" or "wrong"; it's about looking critically at both advantages and disadvantages and providing a fuller picture that allows reviewers to see the rationale behind the proposal, rather than just the personal gut feeling or preference.

Copy link
Author

Choose a reason for hiding this comment

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

We discussed this in #matrix-spec - to summarize, HTML is already used elsewhere, clients already have to deal with it when editing messages, and creating a new format explicitly for this is unnecessary.

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid that doesn't run for a summary of pros and contras, it's more a recap of "why it should be HTML". I'd rather expect advantages and disadvantages of using HTML, plain text or some middle ground. For starters, as also mentioned in #matrix-spec, support of (especially long-form) HTML editing on mobile clients is quite limited - that's a clear disadvantage of HTML. On the other hand, using plain text next to HTML as a kind of middle ground almost doubles the size of the description event (and descriptions are expected to be longer than usual messages), and having both representations doesn't solve the issue of editing the HTML. The advantage of Markdown as a middle ground would be being quite handy for entry even in plain text but a disadvantage is that it's not quite uniformly supported across platforms and SDKs and besides it would be the first case of officially endorsing it as a markup language in Matrix. And so on.

What I'm aiming at is that it cannot be simplified down to "it's everywhere so just deal with it".

proposals/2875-roomdescriptions.md Outdated Show resolved Hide resolved
While this msc is in development, the event should be sent as `cat.blob.msc2875.description` instead of `m.room.description`.

# Alternatives
`m.room.pinned_events` can be used, though not all clients may have access to pinned events, causing issues in many rooms, as well as less consistent display, usage, and support.
Copy link
Member

Choose a reason for hiding this comment

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

I agree; lack of adoption of something already specced is a rather poor argument for speccing yet another event type. The relevant argumentation against using m.room.pinned_events, however, would probably be the intentionally longer form of the description text, compared to that in message events. The UI to enter messages in most prominent clients is not exactly facilitating that longer form.

blobcat-mewmew and others added 3 commits November 23, 2020 15:58
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
@richvdh richvdh deleted the branch matrix-org:master August 27, 2021 18:24
@richvdh richvdh closed this Aug 27, 2021
@turt2live turt2live added abandoned A proposal where the author/shepherd is not responsive and removed proposal-in-review labels Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned A proposal where the author/shepherd is not responsive kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants