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

Edit unsent messages #3097

Merged
merged 8 commits into from
Jun 13, 2019
Merged

Edit unsent messages #3097

merged 8 commits into from
Jun 13, 2019

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Jun 12, 2019

With relations to unsent events supported in matrix-org/matrix-js-sdk#947, this allows editing unsent messages in the UX.

First, it considers pendingEvents while arrow-upping/downing between events to edit.

Second, it does some hackery to allow an uninterrupted experience when the remote echo arrives. When editing an unsent event and the remote echo arrives, the editor will get unmounted and mounted again under a new event tile (with a new key=eventId).

The editor state (current editor model and caret position) needs to be transferred from one editor instances to the new to allow uninterrupted editing. As the editor model depends on the messageeditor react component to be able to trigger autocomplete, we're serializing the model and deserializing it again in the new component.

To keep this state around while the editor is torn down, a new class, EditorStateTransfer, is created, which is passed down from the TimelinePanel all the way down to the MessageEditor, and only replaced when the edit_event action (which is what triggers an edit) is received.

The local echo of editing an unsent event doesn't work yet, as the whole relations infrastructure is implemented in the timelineset, and not present in the pendingEvents... Once the remote echo for the edit arrives, everything falls into place though.

Implements element-hq/element-web#9860
Fixes element-hq/element-web#9864
Needs matrix-org/matrix-js-sdk#947

@bwindels
Copy link
Contributor Author

Karma tests are failing for reasons unrelated to this PR. Will have a look to fix them in a different PR.

@bwindels bwindels marked this pull request as ready for review June 13, 2019 09:13
@bwindels bwindels requested a review from a team June 13, 2019 10:40
@dbkr dbkr requested review from dbkr and removed request for a team June 13, 2019 10:43
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

lgtm - only thing I wondered about was why have a separate class for transferring state between editors rather than just passing the model between them.

src/utils/EditorStateTransfer.js Outdated Show resolved Hide resolved
@bwindels
Copy link
Contributor Author

bwindels commented Jun 13, 2019

lgtm - only thing I wondered about was why have a separate class for transferring state between editors rather than just passing the model between them.

Because things like PillCandidatePart have indirect references to the MessageEditor react component to open the auto completion, hence the (de)serialization. Also, we need to pass the caret. And I didn't want to have to pass two props all the way down from TimelinePanel to MessageEditor so I grouped it in a new class.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

editing with arrow-up doesn't take last message in room into account
2 participants