Add notion of isDismissing and gate onReady() callbacks on it#243
Merged
xiphirx merged 2 commits intoKeepSafe:masterfrom Jan 7, 2018
Merged
Add notion of isDismissing and gate onReady() callbacks on it#243xiphirx merged 2 commits intoKeepSafe:masterfrom
xiphirx merged 2 commits intoKeepSafe:masterfrom
Conversation
There was a race condition I came across where a view could be animating out at the same time as the animation for dismissing the tap target. If the view animated out before the taptarget, it would trigger a layout again, and potentially redisplay the taptarget again. This gates onReady callbacks against the notion of "isDismissing". Tested locally in my project and confirmed this resolves the issue.
xiphirx
requested changes
Jan 7, 2018
Collaborator
xiphirx
left a comment
There was a problem hiding this comment.
lgtm, one small change needed.
| // (like when FLAG_LAYOUT_NO_LIMITS is specified) | ||
| topBoundary = Math.max(0, rect.top); | ||
| bottomBoundary = Math.min(rect.bottom, displayMetrics.heightPixels); | ||
| if (!isDismissing) { |
Collaborator
There was a problem hiding this comment.
Theres no need to update the text layouts if we're dismissing, so we can move this up one line. We can also invert it and early return so we dont unnecessarily indent everything else.
xiphirx
approved these changes
Jan 7, 2018
ganny26
pushed a commit
to wa2do/TapTargetView
that referenced
this pull request
Aug 10, 2018
…fe#243) * Add notion of isDismissing and gate onReady() callbacks on it There was a race condition I came across where a view could be animating out at the same time as the animation for dismissing the tap target. If the view animated out before the taptarget, it would trigger a layout again, and potentially redisplay the taptarget again. This gates onReady callbacks against the notion of "isDismissing". Tested locally in my project and confirmed this resolves the issue. * Return early (and earlier!)
ganny26
added a commit
to wa2do/TapTargetView
that referenced
this pull request
Aug 10, 2018
* Add startWith and startAt to TapTargetSequence (KeepSafe#228) Previously it was cumbersome to restore state in a TapTargetSequence after rotation due to the fact that you needed to manually modify the list of targets you pass in to get back to your desired target. Now the class supports starting from a specified target id or position in the queue, allowing callers to keep a static list of targets for their sequence. * Added Support for React Native Projects (KeepSafe#230) * Update README.md * Update README.md * Update README.md * Update README.md * Add notion of isDismissing and gate onReady() callbacks on it (KeepSafe#243) * Add notion of isDismissing and gate onReady() callbacks on it There was a race condition I came across where a view could be animating out at the same time as the animation for dismissing the tap target. If the view animated out before the taptarget, it would trigger a layout again, and potentially redisplay the taptarget again. This gates onReady callbacks against the notion of "isDismissing". Tested locally in my project and confirmed this resolves the issue. * Return early (and earlier!) * Fix NPE when dismiss is called immediately (KeepSafe#244) Small "race" condition that shouldn't crash. Fixes KeepSafe#240 * 1.11.0 * Fix "sometimes" typo (KeepSafe#246) * Update Gradle version (KeepSafe#253) * Disable interaction during expand animation (KeepSafe#248) * Fix build (KeepSafe#256) * Update to latest support library + SDK version (KeepSafe#257) Was pretty outdated previously. * Use api configuration for public dependencies. (KeepSafe#258) Some support annotations and the v7 support Toolbar are both part of the public api, so they need to be specified as `api` in order for them to transitively picked up by downstream dependants. * Update build tools to most recent ones (KeepSafe#261) * update build tools to most recent ones * revert to last stable version and use gradle 4.5 * Use central build variables (KeepSafe#264) * in module "taptargetview" buildToolsVersion was missing und request an install of 26.0.2 * variable SupportLibraryVersion was just in one module * Handle dismiss before show * Replacement of "compile" with "implementation" (KeepSafe#291) As "compile" is going to be deprecated end-2018, this PR replaces with "implementation" that is the new usage. * Fix NPE from dismissal before determining center (KeepSafe#298) Fixes KeepSafe#279 * Fix not being able to cancel on last target (KeepSafe#299) Simplifies an early return + properly unsets currentView. Fixes KeepSafe#294 * Fix decor related issues (KeepSafe#300) This fixes issues relating to transparent nav bars, status bars and multi window mode. The library will now detect when the window it is attached to has a transparent system bar and properly adjust its clip bounds. Fixes KeepSafe#233 * Release 1.12.0 * Updated readme to reflect latest version
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a race condition I came across where a view could be animating out at the same time as the animation for dismissing the tap target. If the view animated out before the taptarget, it would trigger a layout again, and potentially redisplay the taptarget again. This gates onReady callbacks against the notion of "isDismissing". Tested locally in my project and confirmed this resolves the issue.