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 a crash after removing the view annotation if view has an attached animation or transition #1831

Merged
merged 4 commits into from
Nov 14, 2022

Conversation

kiryldz
Copy link
Contributor

@kiryldz kiryldz commented Nov 14, 2022

Summary of changes

Opened instead of #1829 to let CI pass.
Fixes: #1723

  1. remove view annotation from currentlyDrawnViewIdSet as well
  2. on edge cases (view has animation or transition) onViewDetachedFromWindow will not be called on view removal from viewAnnotationsLayout, so make sure we call it to cleanup viewTreeObserver listener

User impact (optional)

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Run make update-api to update generated api files, if there's public API changes, otherwise the verify-api-* CI steps might fail.
  • Update CHANGELOG.md or use the label 'skip changelog', otherwise check changelog CI step will fail.
  • If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@kiryldz kiryldz added the bug 🪲 Something isn't working label Nov 14, 2022
@kiryldz kiryldz requested a review from a team as a code owner November 14, 2022 09:18
@kiryldz kiryldz self-assigned this Nov 14, 2022
mfazekas and others added 2 commits November 14, 2022 11:19
1. remove view annotation from currentlyDrawnViewIdSet as well
2. on edge cases (view has animation or transition) onViewDetachedFromWindow will not be called, so make sure we call it on removal to cleanup viewTreeObserver listener
@kiryldz kiryldz changed the title Fix view annotation removal if has an attached animation Fix view annotation removal if has an attached animation or transition Nov 14, 2022
@kiryldz kiryldz force-pushed the view-annotation-removal-fixes branch from b00c3eb to 3622b86 Compare November 14, 2022 09:28
@kiryldz kiryldz changed the title Fix view annotation removal if has an attached animation or transition Fix a crash after removing the view annotation if view has an attached animation or transition Nov 14, 2022
@kiryldz kiryldz force-pushed the view-annotation-removal-fixes branch from 3622b86 to f504c4c Compare November 14, 2022 09:30
@kiryldz kiryldz merged commit 395d262 into main Nov 14, 2022
@kiryldz kiryldz deleted the view-annotation-removal-fixes branch November 14, 2022 15:56
kiryldz added a commit that referenced this pull request Nov 14, 2022
* fix: view annotation removal edge cases causes crash

1. remove view annotation from currentlyDrawnViewIdSet as well
2. on edge cases (view has animation or transition) onViewDetachedFromWindow will not be called, so make sure we call it on removal to cleanup viewTreeObserver listener

* Add comment

* changelog

* Add test

Co-authored-by: Miklós Fazekas <mfazekas@szemafor.com>
pengdev pushed a commit that referenced this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash during removal of view annotation - Cannot get annotation options for id: '42', it does not exist.
4 participants