Skip to content

Update all the placeholders on content change #1032

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

Merged
merged 2 commits into from
Feb 24, 2023

Conversation

planarvoid
Copy link
Contributor

@planarvoid planarvoid commented Feb 21, 2023

Fix

The PlaceholderManager contained an optimization that would only update placeholders after the selection start on content change. This works normally fine. However, there is an issue when typing with gestures. Since the inserted word moves the selection start further, the placeholder right after the word wasn't updated. As a fix I've removed this optimization. If we notice any performance issues, we can be smarter about it (and for example only update everything after the previous linebreak).

Test

  1. Set up the MainActivity with the PlaceholderManager. For this you need to initialize the manager and register it with Aztec. Create a ImageWithCaptionAdapter instance and register it with the PlaceholderManager. Replace the EXAMPLE_HTML content with something like <placeholder type=\"image_with_caption\" src=\"and_valid_image_url\" caption=\"test\"></placeholder>
  2. Run the Aztec
  3. Notice only the image with caption visible
  4. Click before the image
  5. Type anything with a gesture
  6. Notice the text is no longer under the image
  7. Notice the images jumps to the next line.
aztec.mp4

Review

@khaykov

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@planarvoid planarvoid added the bug label Feb 21, 2023
@planarvoid planarvoid requested a review from khaykov February 21, 2023 12:57
Copy link
Contributor

@khaykov khaykov left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, @planarvoid ! I confirmed the issue and that the fix works 👍 There is some problem with CI, but feel free to merge it after it is resolved.

@planarvoid planarvoid enabled auto-merge February 24, 2023 12:04
@planarvoid planarvoid merged commit 6355137 into trunk Feb 24, 2023
@planarvoid planarvoid deleted the fix/placeholder-update-after-gesture-writing branch February 24, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants