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

Fix Rich Text Editor issues: save formatted text in draft and keep formatting when switching between modes #8743

Merged
merged 5 commits into from
May 28, 2024

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Jan 31, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • First commit: When using the RichTextEditor, store the formatted text into the Draft, to be able to restore it correctly.
  • Second commit: Keep the formatted text into memory to be able to keep it when switching between composer modes.

Motivation and context

Avoid formatting loses.

Closes #7466

Screenshots / GIFs

FormattedDraft

Tests

  • Enable the RichTextEditor

  • Add some formatted content to the composer, and do not send the message

  • go back to the room list

  • open the room with the draft

  • observe that the text is formatted as before.

  • swipe a previous message to start replying to it

  • observe that the formatting is not lost when doing that

  • Disable the RichTextEditor

  • observe that draft feature works like before (so no regression)

As a limitation, a draft saved with the RTE enabled can be restored a bit wildly once the RTE has been disabled. But I think this is an acceptable limitation. We may want to clear all the drafts when toggling the RTE, but user data will be lost.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@bmarty bmarty requested review from a team and ganfra and removed request for a team January 31, 2024 17:47
@ganfra
Copy link
Contributor

ganfra commented Feb 6, 2024

It looks like it's breaking some computation with the send button visibility... There is some scenarios where the send button never shows up. On develop I can't reproduce, but the send button is blinking when typing text (with RTE enabled).

@bmarty
Copy link
Member Author

bmarty commented Feb 7, 2024

There is some scenarios where the send button never shows up

I cannot repro the issue of send button not visible. Can you be more precise about the scenario to repro please?

@giomfo
Copy link
Member

giomfo commented Apr 12, 2024

It looks like it's breaking some computation with the send button visibility... There is some scenarios where the send button never shows up. On develop I can't reproduce, but the send button is blinking when typing text (with RTE enabled).

@ganfra what is the scenario where the send button disappears? about the blinking, I think this was fixed by #8770.
What is blocking this PR?

@@ -151,7 +150,7 @@ class MessageComposerViewModel @AssistedInject constructor(

private fun handleOnTextChanged(action: MessageComposerAction.OnTextChanged) {
val needsSendButtonVisibilityUpdate = currentComposerText.isBlank() != action.text.isBlank()
currentComposerText = SpannableString(action.text)
currentComposerText = action.text
Copy link
Contributor

Choose a reason for hiding this comment

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

@bmarty Why are u doing that?
As it uses the CharSequence from the editor, it'll break the needsSendButtonVisibilityUpdate as currentComposerText will always be equals to action.text.
Reverting the change still allow to keep formatting when switching mode as far as I see.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, it is a mistake to change this. Reverted in 2395d72

@ganfra ganfra merged commit 22f69ec into develop May 28, 2024
11 of 12 checks passed
@ganfra ganfra deleted the feature/bma/editorIssue branch May 28, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Labs: Formatting on text is lost when returning to the room list in the new WYSIWYG editor
3 participants