-
Notifications
You must be signed in to change notification settings - Fork 206
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
Initial MSC3266 implementation with tweaks for displaying Spaces #219
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure the code is fine, but the mix of tabs and spaces makes this aggravating to review. Please fix it across the whole project, as I can see historical diffs with tabs/spaces issues as well - I'd rather one PR be overloaded than review this indentation nightmare.
Going to hold off landing this until Synapse has support for it, as it does add an additional API hit so a tiny bit of delay to HSes without support for this API - it is negligible but we gain nothing from landing it early |
The Synapse MSC is getting close and I'm going on holiday, merging now, no rush on deploy, will create a card re deploying closer to go live |
FYI the required APIs are now live on matrix.org, it sounds like we'll want to wait for it to be in a release version before deploying though since (I think) matrix.to hits various homeservers? |
the idea is that the "use matrix.org?" dialog should feel no more or less invasive than a GDPR click-thru fwiw. previews are pretty important UX though - in terms of room name, avatar, etc, otherwise it could just come up with a raw ID which is horrific. |
Fixes #213
Fixes #214
Implements matrix-org/matrix-spec-proposals#3266