square/Paralayout

Consider better name for cases in TargetAlignmentBehavior

NickEntin opened this issue · 5 comments

We introduced a TargetAlignmentBehavior enum in #72 to control how views are aligned, in order to address the issues raised around aligning to scroll views in #54. The case names right now are a bit confusing, and I think we can do better, but I'm not sure what would be the clearest naming.

I wonder if it would be better to name these based on which coordinate space it's using. Something like:

/// Align to the position in the target view's superview's coordinate space, i.e. align to the target view's
/// bounds ignoring any origin offset. This is most commonly used when aligning sibling views in a hierarchy.
case targetSuperviewCoordinateSpace

/// Align to the position in the target view's coordinate space, i.e. align to the target view's bounds
/// including any origin offset. This is most commonly used when the target view is in the receiver's
/// superview chain.
case targetCoordinateSpace

Should we drop the "target" prefix?

I agree that repeating "target" in the enum case and case names feels repetitive, but I think it's correct when you read it out as a sentence.

sourceView.align(.rightCenter, to: targetView, .leftCenter, targetAlignmentBehavior: .targetSuperviewCoordinateSpace)

Align the source view's right center position to the target view's left center position in the target view's superview's coordinate space.

The "target" in the enum name says you're controlling how the target view's position is calculated. The "target" in the case name tells you which view's superview we're talking about.

Maybe I'm overcomplicating this? But I think when you write in out in a sentence like that it makes sense.

That's a valid point. Perhaps we could change the parameter name to targeting:

In practice I haven't heard anything about this causing confusion over the last year and a half, so I'm going to close this issue for now.