matrix-org / matrix-rich-text-editor

Matrix Rich Text Editor
https://matrix-org.github.io/matrix-rich-text-editor/
Apache License 2.0
92 stars 23 forks source link

[Android] Add workarounds for ignoring state restoration and invalid line count #864

Closed jmartinesp closed 10 months ago

jmartinesp commented 10 months ago

I found 2 more bugs in the interop between Compose and the AndroidView:

  1. When restoring state in RealEditor's AndroidView, for some reason if there is a single NSBP char at the end of the HTML it'll just remove it, and it can cause crashes because the selection may become invalid. - this was fixed in Rust. In any case, I believe this state restoration is not needed because the state is kept at the ComposerModel and the text and selection are restored in the EditorEditText.onRestoreInstanceState method. This should fix the crashes we're seeing in EXA when a mention is added.
  2. Related to the previous issue, the factory contents run before this onRestoreInstanceState runs, so there's no way to know if we're dealing with a restoration, AFAICT, so by the time we're adding the text changed listener on the factory the state hasn't been restored and the lineCount will always be 0, which in turn will trigger a recomposition of the whole component, and then another one once we have the right lineCount = 1 value. I changed the code to only take into account lineCount if it's > 0, which should fit our usage since we don't care about value 0 (the edittext being empty).
codecov-commenter commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b9fca08) 89.40% compared to head (73430cb) 89.35%. Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #864 +/- ## ========================================== - Coverage 89.40% 89.35% -0.06% ========================================== Files 118 86 -32 Lines 16845 15271 -1574 Branches 635 0 -635 ========================================== - Hits 15061 13646 -1415 + Misses 1760 1625 -135 + Partials 24 0 -24 ``` | [Flag](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | `?` | | | [uitests-ios](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | `?` | | | [unittests](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | `89.35% <ø> (+1.32%)` | :arrow_up: | | [unittests-ios](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | `?` | | | [unittests-rust](https://app.codecov.io/gh/matrix-org/matrix-rich-text-editor/pull/864/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org) | `89.35% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=matrix-org#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jmartinesp commented 10 months ago

Can I get another review for this PR? I added tests for checking the text + selection restoration, as well as keeping the right line count. I also tested it on EXA and it seems to work fine.

sonarcloud[bot] commented 10 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information