Skip to content

Conversation

@planarvoid
Copy link
Contributor

@planarvoid planarvoid commented Jun 13, 2022

Fix

There is an issue where there is a multiline text element before a placeholder. The custom method that was calculating line for offset doesn't take that into account. I've replaced it with the default getLineForOffset and it seems to work well now.

In addition to this I've also made some of the functions in the PlaceholderManager suspended to improve performance and allow async computation

Test

  1. Enable placeholders and add a multiline text followed by a placeholder to the Example field
  2. Run the app
  3. Notice the placeholder is drawn correctly

Review

@danilo04

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 Jun 13, 2022
@planarvoid planarvoid requested a review from danilo04 June 13, 2022 08:07
@planarvoid planarvoid self-assigned this Jun 13, 2022
Copy link
Contributor

@danilo04 danilo04 left a comment

Choose a reason for hiding this comment

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

This works as expected @planarvoid. Thank you for fixing the bug and also adding support to prefetch the views concurrently. I was thinking about how to do this too but were a step ahead 🎉 !!

@danilo04 danilo04 merged commit 2ff653a into trunk Jun 15, 2022
@danilo04 danilo04 deleted the fix/placeholder-position-bug-after-multiline-text branch June 15, 2022 08:28
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.

3 participants