Redefining SmallerPolicy about zoomlayout

[markusressel]

Sounds like some really nice improvements!
Glad I can help 😄

[markusressel]

Great thoughts 👍
I will try my best to add to this discussion 😄

Transformations

As said elsewhere, transformation is not the best name ever, but that's what we have.
The transformation defines the engine resting position. It is a keyframe that is reached at certain points, like at start up or when certain methods are called (setContentSize or setContainerSize with applyTransformation = true).

[...]

As it is defined now, transformation is a purely zoom thing.

Imo this is perfectly valid behavior. The method name is a little uninformative but this can easily be solved by documentation right in the code (like f.ex. the exact text you wrote here).
Although I want to emphasize that I don't think a "breaking change" release would be that bad. I mean the programming language of the whole code changed... Although this doesn't imply a major release version I could understand it if some other library were to do this and renaming things that have unfitting names will only make it easier for future developers using this library to understand how things work and that can only be a good thing - both for (dev-)users and contributors.

Transformation Gravity

Some transformations / aspect ratios might leave with a situation where, after applying the transformation, part of the content is cropped. In these cases, the engine could align the content to different parts of the container, so the transformation gravity applies. This is a purely pan thing.

Error: this gravity does not apply when the transformation leaves the content smaller than the container. Only the opposite. This feels wrong but also right, in that, the 'smaller policy' is a behavior policy that must apply always, not just at transformation time. This makes me think that this method should not exist at all.

What do you mean should not exist? The transformationGravity?
Well one could argue that using a different alignment to initialize the content than the one used as smallerPolicy might lead to inconsistent/unexpected behavior from the user's point of view - but imo it is also perfectly valid to initially align a (cropped) image to the top left but center it when zooming out too far. Giving the developer the option to choose which is best for his use case would be my preferred option.

When content is smaller than container

This can happen in various situations:

• With TRANSFORMATION_NONE and a content that is actually smaller
• With TRANSFORMATION_CENTER_INSIDE, one of the axes will probably be smaller
• Unless minZoom is very high, pinching out can always make content smaller than the container

We've been ignoring this case until it was brought up in #71 . In this situation, I see that the engine might actually do two things:

• Let the content be moved around freely ( = SMALLER_POLICY_NONE)
• Align the content to some part of the container according to some gravity flag

I can't think of any third option. If you can, please say.

If the content is not aligned automatically (SMALLER_POLICY_NONE) and no transformationGravity is specified how will the content be positioned then on initial transformation? Are the options available for the transformationGravity different from the alignment options (as proposed below) meaning there is no TRANSFORMATION_GRAVITY_NONE? The TRANSFORMATION_CENTER_INSIDE transformation - although it has a CENTER in it - should not effect pan. Would a dev be able to position the content in an exact location aka coordinates? Is this even desirable?

But if these are the only two options...

Proposal: Alignment (Gravity)

Remove SmallerPolicies. Create ALIGNMENT_ integer flags as always. Then create alignment XML attribute and setAlignment method.

Possible values should be

• ALIGNMENT_NONE : corresponds to SMALLER_POLICY_NONE
• ALIGNMENT_TOP, ALIGNMENT_BOTTOM, ... : aligns to that specific side of the container

Docs should made clear that alignments only apply when content is smaller than the container, because forcing an alignment when content is bigger, makes the hidden content unreachable. This is actually obvious, not a design decision. When content is bigger you must be able to pan around.

Sounds good 👍

About the name

I was also tempted to call this setGravity, I see arguments on both sides. Let me know what you think.

I would also tend to use alignment because at least in my head the gravity properties of views in Android are fixed properties in the sense of "every android view has this property" and they always apply. Reusing it in the engine which does not use it in the exact same way the gravity on views/layouts is used would be confusing to me and this whole "smaller/bigger if else" stuff means it does not apply equally at all times.

EDIT: I though about using contentGravity instead but the fact that the original gravity constants would not match the exact use case here I still think alignment is the better option but I'm still hesitant. Maybe use contentAlignment to make it more clear that it is the contentView that is actually aligned by this flag and not the ZoomLayout view itself?

• At the same time this is confusing because we will also have GRAVITY_NONE and also we do not support all the gravity fields. A gravity "top|bottom" or "fill_horizontal" is ignored by the engine.
• With alignment we are free to do more fancy stuff. For example, "none_horizontal|top", "center_horizontal|none_vertical". We could have none as a flag and apply it to single axis.

I do prefer alignment.

These are good thoughts. The GRAVITY_NONE case is a good example why the android view gravity constants are not sufficient.

I don't think using constants like none_horizontal or none_vertical does make sense though since it should be the default behavior if nothing is specified. I for one would expect setAlignment(top) to result in "none_horizontal|top".

About transformationGravity

[...]

So we should be inverting the dependency, make transformationGravity default to the alignment and not the opposite. Because transformationGravity is just a pan, one-time translation applied after the transformation and only when content is bigger. The alignment is a key behavior of the engine, always active.

Sounds good 👍

[natario1]

I don't think a "breaking change" release would be that bad

Not in itself, but I don't see this urge either, because on that occasion we would like to do all the breaking changes needed, you know. We're just not there yet, didn't give any thought...

but imo it is also perfectly valid to initially align a (cropped) image to the top left but center it when zooming out too far.

I also agree, that's what I thought until #71 !

Are the options available for the transformationGravity different from the alignment options (as proposed below) meaning there is no TRANSFORMATION_GRAVITY_NONE

Yes, transformationGravity is a pure android gravity and the name is fine. It makes no sense to have "none" (= content is free to be moved) because content is free to be moved, it's just that keyframe pan, not a policy.

Should we rename none to free? I don't know if you have tried the app with the none smaller policy, but that's basically what it is.

If the content is not aligned automatically (SMALLER_POLICY_NONE) and no transformationGravity is specified how will the content be positioned then on initial transformation?

I would default to center for any unspecified axis? Like none_horizontal|top becomes center_horizontal|top?

I though about using contentGravity instead but the fact that the original gravity constants would not match the exact use case here I still think alignment is the better option but I'm still hesitant. Maybe use contentAlignment to make it more clear that it is the contentView that is actually aligned by this flag and not the ZoomLayout view itself?

I like contentAlignment as XML attribute. But I don't think the Engine method should be named so, or we might have setContentMinZoom() and so on... All methods are basically referred to the content. So I think I would leave alignment in XML as well.

I don't think using constants like none_horizontal or none_vertical does make sense though since it should be the default behavior if nothing is specified. I for one would expect setAlignment(top) to result in "none_horizontal|top".

I was thinking to default to center, but I see your point 👍 well at least we need none (or free) so one can explicitly say setAlignment(FREE).

[markusressel]

Not in itself, but I don't see this urge either, because on that occasion we would like to do all the breaking changes needed, you know. We're just not there yet, didn't give any thought...

Your choice 👍

Yes, transformationGravity is a pure android gravity and the name is fine. It makes no sense to have "none" (= content is free to be moved) because content is free to be moved, it's just that keyframe pan, not a policy.

Should we rename none to free? I don't know if you have tried the app with the none smaller policy, but that's basically what it is.

No I think none is fine then. I just wanted to make sure I understand you correctly which seems to be the case.

I would default to center for any unspecified axis? Like none_horizontal|top becomes center_horizontal|top?

Well reading #71 again this could/should be different for ZoomImageView (where center is good) than ZoomLayout (where top/left should be preferred I guess?). But I would be fine with both since the dev can always change it if they want.

Quote from #71:

I doubt centering the view really fits 99.9% of users as it is not the default behavior on iOS. I’ll be using your library to allow scaling of a spreadsheet-like report. This seems like a common use-case of a zoomable scrollview. The Android ScrollView also does not center it’s content by default and I expected the same behavior.

I like contentAlignment as XML attribute. But I don't think the Engine method should be named so, or we might have setContentMinZoom() and so on... All methods are basically referred to the content. So I think I would leave alignment in XML as well.

Yes you're right, I agree with using just alignment then.

I was thinking to default to center, but I see your point well at least we need none (or free) so one can explicitly say setAlignment(FREE).

Yes but only for both axis at the same time. Using horizontal_none or vertical_none respectively as the default would make it logical that the absence of any specific alignment on any given axis means just "do nothing". Defaulting to xxx_center just forces us to include two additional flags that don't really add much value (imho!).

[natario1]

Well reading #71 again this could/should be different for ZoomImageView (where center is good) than ZoomLayout (where top/left should be preferred I guess?). But I would be fine with both since the dev can always change it if they want.

I agree. Just won't do it now cause it would be a subtle breaking change :-) though IMO that kind of alignment when pinching out is the ugliest thing ever.

Using horizontal_none or vertical_none respectively as the default would make it logical that the absence of any specific alignment on any given axis means just "do nothing".

Yes, let's do this way, you're right.

If you come up with a good name for transformation we might (for now) deprecate transformation and add the new method and leave both. In the long run what I would like to do is:

• Find a good name (let's say keyframe for now)
• Methods would be setKeyframeTransformation and setKeyframeGravity
• Or even setKeyframeZoom (accepting a zoom value or a constant) and setKeyframePan (accepting pan values or a gravity constant)
• Add goToKeyframe() method
• Add makeKeyframe() method: just calls setKeyframeZoom and setKeyframePan with current values. Future keyframe operations will reach this exact position.
• Rename @realZoOm to @zoom : means actual zoom with respect to the content dimensions
• Rename @zoom to @KeyframeZoom : means zoom with respect to the keyframe state
• So finally have ZOOM_TYPE_ZOOM and ZOOM_TYPE_KEYFRAME

The last change IMO makes sense but will change the meaning of getZoom() to mean the opposite type. Docs would then mention that after keyframe is applied the engine applies any constraint (minZoom, maxZoom, alignment...) also.

Will do what we said for now, thanks