Paralayout icon indicating copy to clipboard operation
Paralayout copied to clipboard

Consider better name for cases in TargetAlignmentBehavior

Open NickEntin opened this issue 4 years ago • 4 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.

NickEntin avatar Nov 23 '21 22:11 NickEntin

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

NickEntin avatar Dec 20 '21 21:12 NickEntin

Should we drop the "target" prefix?

cocoalabs avatar Dec 20 '21 21:12 cocoalabs

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.

NickEntin avatar Dec 21 '21 23:12 NickEntin

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

cocoalabs avatar Dec 21 '21 23:12 cocoalabs

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.

NickEntin avatar Mar 16 '23 20:03 NickEntin