Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Stop sending reply and edit fallbacks (MSC2781) #6964

Closed

Conversation

aaronraimist
Copy link
Contributor

@aaronraimist aaronraimist commented Oct 17, 2021

Fixes element-hq/element-web#20334
Fixes element-hq/element-web#7874
Fixes element-hq/element-web#7014

Implementation of MSC2781 matrix-org/matrix-spec-proposals#2781


This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://616fbd209150a3addcee4a49--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@aaronraimist aaronraimist requested a review from a team as a code owner October 17, 2021 05:43
@SimonBrandner SimonBrandner added T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements T-Task Refactoring, enabling or disabling functionality, other engineering tasks and removed T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Oct 17, 2021
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.

While the code looks fine, there's ecosystem compatibility to consider on this, including with bots, bridges, other clients, etc.

This PR serves functionally fine for an implementation of the removal, but for the spec side we'd need reassurance that bridges and bots won't break (clients are easier to fix).

@turt2live turt2live added the X-Blocked The PR cannot move forward in any capacity until an action is made label Oct 19, 2021
@HarHarLinks
Copy link
Contributor

while I agree the ecosystem needs to adapt, I think it's much easier to support events without than with fallbacks also for bots just as it is for client( sdk)s. What's the plan to push towards it? Labs flag for this and test around how it goes? Announce to everyone to implement this MSC before it gets merged while retaining compatibility with fallbacks with a deadline?

@turt2live
Copy link
Member

See also: MSC2781

@turt2live turt2live self-requested a review January 20, 2022 23:50
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.

This PR will probably take a while to merge, as it would require us to have suitable support on mobile (android at least doesn't currently render replies properly). The MSC also appears to be stalled, which isn't good :(

For now I think we can keep this PR open because it's not that old, though if the Android/mobile support or MSC take too long then we may revisit this at a later date.

Code-wise this looks fine. Just marking requested changes for the mobile support (and MSC, though to a lesser degree).

@MadLittleMods MadLittleMods added the Z-Community-PR Issue is solved by a community member's PR label Jun 1, 2022
@weeman1337
Copy link
Contributor

weeman1337 commented Aug 10, 2023

Conversion to a draft, as we are still waiting for mobile. We should reopen it if there is a chance to move it forward.

(see Oldest non-draft PR from last Web Weekly)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks X-Blocked The PR cannot move forward in any capacity until an action is made Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
7 participants