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

Show/Hide Blockquote Fix #1594

Merged
merged 9 commits into from Feb 20, 2021
Merged

Conversation

siefkenj
Copy link
Collaborator

@siefkenj siefkenj commented Feb 2, 2021

Fixes race condition that occurs when detecting block quotes to show/hide.

When Conversations is deciding whether or not to put the show/hide button on a block quote, it computes the blockquote's height. This computation only works if the DOM is fully loaded. This code sets a timeout to wait for the DOM to fully load as blockquote detection runs.

A consequence of this is that the height of the iframe cannot be computed right away--the code needs to wait for blockquote detection to finish. This adds complication, and the solution here is probably fragile...I suggest migrating the resizing of the iframe to a well-tested library like https://github.com/davidjbradshaw/iframe-resizer-react

Fixes #1591

@Standard8
Copy link
Collaborator

I was wondering if we might be able to do more with waiting for animation frames etc. However, that might also end up with the same issues.

I'd be happy to for us to try out the iframe resizer. Having code run by others would probably be good here.

As long as we can set all the normal iframe attributes via it, I'm fine with that.

@siefkenj
Copy link
Collaborator Author

siefkenj commented Feb 5, 2021

@Standard8 Were you wanting that to be done in this PR? Truth be told, I haven't used iframe-resizer-react before. Just read about it...

Copy link
Collaborator

@Standard8 Standard8 left a comment

Choose a reason for hiding this comment

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

I think we should investigate iframe-resizer-react, the changes here seem a little fragile, though I don't see a better way of doing them at the moment.

However, lets go with getting this in for now, to hopefully fix the immediate issue, and then we could try that out later.

@Standard8 Standard8 merged commit 488a9d0 into thunderbird-conversations:main Feb 20, 2021
Standard8 pushed a commit that referenced this pull request Mar 13, 2021
* Fixed race condition when mangling block quotes
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.

Quoted messages in the body of the email have stopped collapsing
2 participants