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

Save edited state of a message when switching rooms #6001

Merged
merged 6 commits into from
May 14, 2021
Merged

Save edited state of a message when switching rooms #6001

merged 6 commits into from
May 14, 2021

Conversation

jaiwanth-v
Copy link
Contributor

Fixes element-hq/element-web/issues/17202

edit.mp4

Signed-off-by: Jaiwanth jaiwanth2011@gmail.com
@jaiwanth-v
Save edited state when switching rooms
6f98aa0 
Signed-off-by: Jaiwanth <jaiwanth2011@gmail.com>
@jaiwanth-v jaiwanth-v changed the title Save edited state when switching rooms Save edited state of a message when switching rooms May 9, 2021
@t3chguy t3chguy requested a review from a team May 10, 2021 15:42
turt2live
turt2live suggested changes May 11, 2021
View reviewed changes
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.

Overall it seems generally sane, though there's a bunch of unanswered questions which should either get comments or be altered to match the code style.
@jaiwanth-v @turt2live
Update src/components/views/rooms/EditMessageComposer.js
376befd 
Co-authored-by: Travis Ralston <travpc@gmail.com>
@jaiwanth-v
Copy link
Contributor Author

Sorry about the deleted new lines and comments. I don't know why I did that. I'll replace them back.
@jaiwanth-v jaiwanth-v requested a review from turt2live May 11, 2021 07:54
t3chguy
t3chguy reviewed May 11, 2021
View reviewed changes
@jaiwanth-v
Copy link
Contributor Author

jaiwanth-v commented May 11, 2021 •
edited
Loading

Also, should we scroll users to the edited event? The cursor is focused on the edit composer, so it might not be obvious if the message is out of viewport.
turt2live
turt2live approved these changes May 14, 2021
View reviewed changes
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.

seems sensible, thanks! I don't think we need to scroll for the user - the scroll position is already supposed to be saved, so that information would typically carry through on most user interactions.
@turt2live turt2live merged commit 2d2c0d5 into matrix-org:develop May 14, 2021
@robintown robintown mentioned this pull request May 17, 2021
Merged
@SimonBrandner SimonBrandner mentioned this pull request May 29, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants

@jaiwanth-v @turt2live @t3chguy