MSC4482: Ability to bookmark messages#4482
Conversation
Init Bookmark MSC
…y-to-bookmark-messages.md
|
Signed-off-by: xeni [email protected] |
turt2live
left a comment
There was a problem hiding this comment.
This MSC appears to be a work in progress, so I haven't done the normal "implementation requirements" comment yet. When ready, feel free to request such a comment in the SCT Office.
|
I completed the proposal part. Tell me if I'm too verbose @Gernomaly |
|
Love the additions! I am going to elaborate on the alternatives parts later today. Including the comparison that travis suggested 👍 |
| ### New room type | ||
|
|
||
| This MSC introduces a new room type called `m.bookmarks`. | ||
| This kind of room SHOULD only hold some `m.bookmark` events defined below. |
There was a problem hiding this comment.
@Gernomaly I'm not sure anymore about this one.
As said in private, I want to adapt my Refs client so it supports this MSC. However, when I need to create a personal bookmark ex-nihilo (not from an existing Matrix event), I need to send my custom Ref event somewhere, and then reference it with our new m.bookmark "pointer event".
Currently I'm using my own custom room type to store my custom events, so using just this m.bookmarks instead would be nice.
So my proposition is that instead of SHOULD only we put MAY ? Does that sound good ?
There was a problem hiding this comment.
i think should is fine, its not a must and may is too soft imo
There was a problem hiding this comment.
id just stay with should only tbh, its not like the should only prevents you of not adding anything else imo, and from my point of view your events are a kind of bookmark as well :P . To me it its important to at least for the moment keep the scope together a bit and adapt it if needed in the future if the protocol develops into that direction (referencing msc guide)
There was a problem hiding this comment.
Ok I will put back the only then. You're right we should keep it scoped to the generic usecase for now
| } | ||
| ], | ||
| "m.pointer": { | ||
| "roomId": "<roomId>", |
There was a problem hiding this comment.
Since room IDs alone are not routable in Matrix, would you also need a via property possibly? I assume you'd normally already be in the room so maybe not? 🤔
There was a problem hiding this comment.
Our approach of pointers is kinda simple, we are ok if some pointers are just "dead". It should be up to the client to eventually clean up dead pointers (bookmarks pointing to rooms you're no longer in for instance).
Also, I guess if you don't clean a bookmark to a public room you're no longer part of, having the via parameter could help you joining this room back, so I have mixed opinions on this.
| ## General | ||
|
|
||
| If the client support the `m.bookmarks` room type, it should'nt be displayed as a normal room in the regular room list. Instead the client could display bookmarks in dedicated sections of the client. | ||
| The client should ensure that only one `m.bookmarks` room is existing at once. |
There was a problem hiding this comment.
Could an account data event help to let clients store the room ID of the one bookmarks room?
There was a problem hiding this comment.
We imagined that but it would probably require extra work if we need to upgrade the m.bookmarks room at some point.
|
|
||
| ## Room creation | ||
|
|
||
| The room should be created upon first bookmark creation. |
There was a problem hiding this comment.
This feels like it could be an implementation detail. Would there be any issues with letting clients create the room prematurely?
There was a problem hiding this comment.
When we discussed it IRL I remember two arguments :
- waste of ressources
- zero click creation of a room that contains nothing yet, so it could be weird for a user that also uses a client that does not support
m.bookmarksrooms, and thus display the fallback room containing nothing.
We're aware that the first bookmark creation should be a little longer, but that is probably up to the client to make it smooth.
There was a problem hiding this comment.
@Johennes is that a sufficient answer to that question or do you still think we should change it?
rendered