Feature/alignment #76
Conversation
| |`fromTransformation`|The content will respect the gravity parameter of the transformation (which defaults to `Gravity.CENTER` as well).| | ||
| |`none`|The content is free to be moved around inside the container bounds.| | ||
| **Note: alignment does not make sense when content is larger than the container, because forcing an | ||
| alignment (e.g. left) would mean making part of the content unreachable (e.g. the right part).** |
There was a problem hiding this comment.
[...] so it is only applied when the content is smaller than the container (e.g. zoom < 1)
There was a problem hiding this comment.
I will address all these in a while. But wanted to say that zoom < 1 does not necessarily mean that content is smaller than container. It does only with CENTER_CROP or CENTER_INSIDE transformations and even then, it depends on the axis.
Just saying because I have seen this check (zoom < 1F) in the pivot function, and I don't know if it's correct or not. I'm tempted to say that it's not, and I also wonder if that should be changed according to the alignment? Not sure.
There was a problem hiding this comment.
Hmm yes that makes sense. I guess I was lucky in my testing.
Yes I think so too. As i mentioned in the comment there I initially wanted to base it on the transformationGravity but as the alignment will be the new thing after this PR it should be based on it instead.
There was a problem hiding this comment.
Can you explain what that function should return? It's not 100% clear to me, I didn't investigate, but I can try to update
There was a problem hiding this comment.
Well the function decides what pivot point should be used for the zoom animation based on what corner of the screen is currently overscrolled (or will be, with the target zoom applied). I did this because applying the pan fix and the zoom fix at the same time in an animation when overscrolled while zoomed in a bit (specifically overpinching and overscrolling at the same time in a single pinch gesture) didn't result in the expected target position and/or a funky animation. Implementing this was a bit of trial and error tbh and I think this method is only applicable when the content is zoomed in meaning that at most two sides of the content are overscrolled/have visible gray areas. Because of that I added the zoom < 1 condition which just takes the center of the screen as the pivot point when the view has more than two gray areas around it (which doesn't seem to be the correct way of figuring that out). My current guess would be that this should be based on the alignment instead of just using the center when there are more than two gray areas but this whole method (and it's purpose) may require a complete rethinking.
There was a problem hiding this comment.
I understand. Looks like it might take some time, I think I will merge this now and we'll fix later.
Fixes #75