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

Strange/Glitchy navigation animations in Release 1.5.8 #133

Closed
abhijitvalluri opened this issue Sep 11, 2016 · 16 comments
Closed

Strange/Glitchy navigation animations in Release 1.5.8 #133

abhijitvalluri opened this issue Sep 11, 2016 · 16 comments
Assignees
Labels
Milestone

Comments

@abhijitvalluri
Copy link

abhijitvalluri commented Sep 11, 2016

Firstly, thanks for an amazingly detailed library! I am currently using it in my published app for the introduction setup.

Right now, my latest published app uses Release 1.5.5 of your library. This version seems to do fine with regard to the page navigation and the corresponding animations. Firstly, some context on my usage of your library: I have used multiple "SimpleSlide"s for my setup process. Some of these slides need to disable "CanGoForward" as the user is forced to click on the Cta button. The button issues an Intent that launches another app's activity and upon return to my app, in the onActivityResult, I retrieve the particular SimpleSlide, and replace it with an identical slide where "CanGoForward" is set to true. I haven't figured out a way to change a SimpleSlide's "CanGoForward" data member, so I had to replace it with a new Slide object. Then the user can press the forward button and continue.

Starting from 12th second, upon returning from the other app's activity + replacing slide with another that allows forward navigation, this is what happens: When I navigate forward by pressing the forward arrow, rather than a smooth transition, I see a sudden jump to the next Slide. Scrolling seems fine. However, on the next Slide, where "CanGoForward" is false again, scrolling causes the Slide's content to vanish (at 00:18 in 2nd video). Pressing the forward arrow disallows navigation as expected though. Pressing the back arrow causes some strange navigation animations that I can't quite explain.

Could you please look into this as I cannot currently use the latest version due to this strange animation issues?

@daniel-stoneuk
Copy link
Collaborator

daniel-stoneuk commented Sep 11, 2016

This is fixed in the latest snapshot, as I fixed it in commit 2bb3fc3. If you want to use the latest version, replace the dependency with this:
com.heinrichreimersoftware:material-intro:c68005f
That will use the latest commit, but will not change if something else or a new bug is added to the library.

@heinrichreimer
Copy link
Owner

Thanks @daniel-stone for pointing that out. I think I should release a quick bug fix release soon.

@abhijitvalluri
Copy link
Author

abhijitvalluri commented Sep 11, 2016

Thanks @daniel-stoneuk and @heinrichreimer for response. I tried the latest commit by replacing the dependency with the commit at c68005f, but it appears that the exact same bug continues.

@heinrichreimer
Copy link
Owner

@abhijitvalluri could you make another screen recording preferably with a higher frame rate so that we can see the glitch?

@daniel-stoneuk
Copy link
Collaborator

Ah right, sorry for assuming it was fixed. I was having similar issues and that fixed it.

@daniel-stoneuk
Copy link
Collaborator

Can you not just get the slide at an index and then call a public method (after checking instance of) to change the canGoForward variable?

@heinrichreimer
Copy link
Owner

Yep, @daniel-stoneuk is right about that. If you want to change canGoForward FragmentSlide and SlideFragment are your friends. Check out how I implemented the LoginSlide in the sample. It checks wether the user logged in and notifies the IntroActivity that it's canGoForward has changed.

If you don't want to use a FragmentSlide because you only need the default slide style, check out NavigationPolicy. You could store a boolean variable in your shared preferences when someone clicks the CTA button and then call lockSwipeIfNeeded() on your IntroActivity to notify the intro that your canGoForward has changed.

@heinrichreimer
Copy link
Owner

@daniel-stoneuk you can't modify a SimpleSlide.

@daniel-stoneuk
Copy link
Collaborator

Ah yup, I was thinking of FragmentSlide

@heinrichreimer
Copy link
Owner

@daniel-stoneuk nor can you modify a FragmentSlide ;-)

@heinrichreimer
Copy link
Owner

You can only modify the Fragment if you used FragmentSlide.Builder.fragment(Fragment fragment).

@abhijitvalluri
Copy link
Author

abhijitvalluri commented Sep 12, 2016

The behavior with the latest commit was exactly the same as the one with
Release 1.5.8 shown above. However, I have identified the cause of the
problem. I have been using a SimpleSlide and in some cases I decided to set
canGoForward to false, because the user of my app has to interact with the
slide, such as click the CTA button to launch an external app for setup
purposes. When the user finishes the interaction on the slide, I had to
create a new SimpleSlide with canGoForward set to true, because there is no
public method to set its value after building the slide. Only option was to
replace the slide. However, when I use the setSlide() method, it calls
notifyDataSetChanged() which then calls scrollTo(0,0). This sets the data
members mScrollX and mScrollY of the ViewPager to (0,0) as well. This is
the root of the problem.

From here on out, the logic in updatePosition() in
SwipeBlockableViewPager.java goes out the window because the current
slide's mScrollX is no longer currentItem*getWidth() but instead it is 0!
So when that method is called it uses the incorrect and outdated value for
x, i.e. the first parameter, and so I see all that crazy scrolling.

I think there is no way to reliably fix this unless you make sure that
users don't use setSlide() in the middle or may be save the edit to
mScrollX caused by notifyDataSetChanged() in setSlide(). Also, I could have
avoided using setSlide() if there was a public method to set canGoForward
and canGoBackward.

I currently fixed my glitch by using FragmentSlide's instead of the
SimpleSlide and avoiding setSlide(). This made my intro nicer anyway!

Thanks for looking into this!

@heinrichreimer
Copy link
Owner

@abhijitvalluri nice catch. I'll check out this issue with notifyDataSetChanged() next weekend. Have you tried the method I described by using a NavigationPolicy?

@abhijitvalluri
Copy link
Author

@heinrichreimer I did not use NavigationPolicy, but I extended SlideFragment and overrode the canGoForward and canGoBackward methods and defined my own methods to set them as well. And then, I used FragmentSlide.Builder.fragment(Fragment fragment) with my custom SlideFragment.

@heinrichreimer
Copy link
Owner

Yep, that should work

@daniel-stoneuk
Copy link
Collaborator

@heinrichreimer That's what I meant... Of course 😂😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants