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

[WIP] MSC3868: Room Contribution #3868

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

jae1911
Copy link

@jae1911 jae1911 commented Aug 15, 2022

Rendered

Signed-off-by: Aminda Suomalainen suomalainen+git@mikaela.info
Signed-off-by: Jae Lo Presti me@jae.fi

jae1911 and others added 2 commits August 15, 2022 23:31
Co-authored-by: Aminda Suomalainen <suomalainen+git@mikaela.info>
@jae1911 jae1911 changed the title [WIP] MSCXXX: Room Contribution MSC3868: Room Contribution Aug 15, 2022
@turt2live
Copy link
Member

turt2live commented Aug 15, 2022

(converting this to a draft given TODO-style comments within the text)

Edit: or not, because for some reason it won't let me.

@turt2live turt2live changed the title MSC3868: Room Contribution [WIP] MSC3868: Room Contribution Aug 15, 2022
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API 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. labels Aug 15, 2022
@@ -0,0 +1,55 @@
# MSC3868: Room Contribution
Copy link
Contributor

Choose a reason for hiding this comment

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

Not actually relevant for this MSC (and imho not a blocker), but I want to have it noted at an area we can link to later:

This seems also a pretty good thing to have when we have profiles as rooms eventually :) Basically having "personal links" via this in a nice way.

@jae1911 jae1911 marked this pull request as draft August 15, 2022 21:21
Mikaela and others added 4 commits August 16, 2022 00:29
Signed-off-by: Aminda Suomalainen <suomalainen+git@mikaela.info>
Signed-off-by: Aminda Suomalainen <suomalainen+git@mikaela.info>
…light dependency

Co-authored-by: Jae Lo Presti <me@jae.fi>
Signed-off-by: Aminda Suomalainen <suomalainen+git@mikaela.info>
@MayeulC
Copy link
Contributor

MayeulC commented Aug 16, 2022

A lot of GitHub projects display badges. It might be interesting to allow images as well as urls, possibly with an extra key: "badge-url" : "mxc://something"

@jae1911
Copy link
Author

jae1911 commented Aug 16, 2022

A lot of GitHub projects display badges. It might be interesting to allow images as well as urls, possibly with an extra key: "badge-url" : "mxc://something"

I find that to be a good idea, however, after a discussion with @Mikaela we are a bit concerned over some points:

@MayeulC
Copy link
Contributor

MayeulC commented Aug 17, 2022

To be clearer:

  • I think a regular URI should not be identified, only mxc content
  • I did not initially think of "dynamic" badges, mostly something like these instead:
image image image
image image etc

You are right though, it would be tempting to upload a lot of images for dynamic badges.
It would be simpler if mxc links were content-based #3468, as that would de-duplicate them.

For cleanup, there is still the admin API: https://matrix-org.github.io/synapse/latest/admin_api/media_admin_api.html#delete-local-media-by-date-or-size and purge remote media. Those files should be relatively small, but I don't really see a solution short of allowing a background-image and formatted_body, which sounds relatively complex to implement.

Other changes:
 - Added comment on possible benefit from MSC3468

Co-authored-by: Aminda Suomalainen <suomalainen+git@mikaela.info>
@jae1911
Copy link
Author

jae1911 commented Aug 17, 2022

Thanks for the input @MayeulC and what you said makes sense.
Added it to the MSC!

@MayeulC
Copy link
Contributor

MayeulC commented Aug 18, 2022

Looks good!
Maybe advise against uploading new content too frequently?
I am not entirely sure about images:

  • maybe base64 encoding would be better?
  • is it necessary to specify width/height limits? Or is it implied that clients will only likely draw them with small dimensions, therefore it is advised to use images that work well at small size? Usually with such images, aspect ratio is the issue, as images have a max height of ~1em and no real max width (though in practice they do).

I assume people will just pick something that looks good on their client.

@jae1911
Copy link
Author

jae1911 commented Aug 18, 2022

I assume as well that clients will format it the best it looks for them and will probably fix height and width limits on their own.

I'm gonna look into base64 but I think the MXC URIs are solid and the most practical choice for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API 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