-
Notifications
You must be signed in to change notification settings - Fork 3
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
Added some width/height constraints #3
Conversation
Why did you make this change? These constraints were missing from the library and I needed them in my project so I'm pushing them up.
]) | ||
} | ||
|
||
open func matchWidthToHeight(constant: CGFloat = 0.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was previously named square(), but I didn't like how the name didn't quite follow the rest of library. I'm open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, makeSquare
, equalSizeAttributes
, equalDimensions
, matchDimensions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equalizeDimensions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I like makeSquare as it's the easiest to understand
@@ -7,6 +7,7 @@ | |||
objects = { | |||
|
|||
/* Begin PBXBuildFile section */ | |||
51E19A9D1E2A82F900998194 /* ExpandFromWidth.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51E19A9C1E2A82F900998194 /* ExpandFromWidth.swift */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should rename this file ExpandFromSize.swift
and have it include expanding from both Width and Height.
@claude also noticed that you didn't add any test coverage. I know we don't have great test coverage to start. But, I think we should start requiring test coverage. |
Why did you make this change? Because we needed tests and it made more sense to rename the things
@cyphactor this PR has been updated to address your comments |
Why did you make this change?
These constraints were missing from the library and I needed them in my project
so I'm pushing them up.