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

FlipHorizontal and FlipVertical changes incorrect IMO #52

Open
pandasys opened this issue Jul 8, 2016 · 1 comment
Open

FlipHorizontal and FlipVertical changes incorrect IMO #52

pandasys opened this issue Jul 8, 2016 · 1 comment

Comments

@pandasys
Copy link

pandasys commented Jul 8, 2016

Don't have time to do a proper change, test, and pull request so I'm just going to write this issue. If no one fixes, I'll come back to make the change and pull request after my app release.

Problem: those modifications have the side effect that if I change transformations after a vertical or horizontal flip, when I swipe the other direction, the off screen view, which is now on screen, remains hidden.

I believe the framework already (sort of) supported the necessary change to remedy this. For anyone that runs into the problem, make the following change to ABaseTransformer:
if (hideOffscreenPages() && (position <= -1f || position >= 1f)) { page.setVisibility(View.INVISIBLE); page.setAlpha(0f); // page.setEnabled(false); } else { // page.setEnabled(true); page.setAlpha(1f); page.setVisibility(View.VISIBLE); }
And then you can remove the onPostTransform from FlipHorizontal and FlipVertical.

In doing other work I did a limited amount of research on alpha vs visibility. I found a post from Romain Guy saying to use visibility and not alpha to hide/show as it's more efficient. The original defects were due to the framework using alpha to hide. The partial fixes switched to using visibility to prevent interaction with "off screen" pages, as it should be IMO. Problem is the side effect I found. Moving this into the framework itself fixes both these transformations and future transformations.

I left the alpha change in as it's necessary to clean up after some transformations. I don't think the setEnabled is necessary and it's commented out in my copy.

To the original developer that fixed the Flip transformation, thank you! I'd been having a problem being unable to scroll after a transformation and not taking the time to debug. Your fix helped very much.

@ToxicBakery
Copy link
Owner

I imagine what we really need is unit tests so swiping can be proven to work as intended for each transform and when swapping between each transform.

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

No branches or pull requests

2 participants