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

Set bounds on all new motion controllers #588

Merged
merged 3 commits into from
Sep 12, 2023

Conversation

zsoltk
Copy link
Contributor

@zsoltk zsoltk commented Sep 12, 2023

Description

Observed bug

Sample app scenario:

  1. Moved Promoter to inside Spotlight, rather than BackStack
  2. Embedded component works at first, but when moved off screen then re-added to composition, it looks visually broken

Why this happens

  1. With BackStack, Promoter was popped and recreated the next time; however, with Spotlight it was kept alive

  2. The embedded AppyxComponent receives updateContext when re-entering the composition

  3. This leads to its MotionController being recreated (without any knowledge of TransitionBounds)

  4. Since the component cached the last TransitionBounds, and the new one passed to it is exactly the same, the if check does not set it on the new MotionController instance:

    override fun updateBounds(transitionBounds: TransitionBounds) {
        if (transitionBounds != this.transitionBounds) {
            this.transitionBounds = transitionBounds
            _gestureFactory = gestureFactory(transitionBounds)
            _motionController?.updateBounds(transitionBounds)
        }
    }

    Which is fine that it's skipped, but we shouldn't rely on this being called either.

  5. If the size of the bounds is used to calculate any TargetUiState values (as is the case with Promoter), those will be messed up.

Fix introduced

Always set bounds on the new MotionController instance.

Check list

  • I have updated CHANGELOG.md if required.
  • I have updated documentation if required.

@zsoltk zsoltk added bug Something isn't working appyx-interactions labels Sep 12, 2023
@zsoltk zsoltk added this to the 2.0 milestone Sep 12, 2023
@zsoltk zsoltk merged commit e44deb5 into bumble-tech:2.x Sep 12, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appyx-interactions bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants