Skip to content
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

Create ViewDistributionBuilder #117

Merged
merged 7 commits into from
Apr 23, 2024

Conversation

dfed
Copy link
Collaborator

@dfed dfed commented Apr 21, 2024

Resolves #116

Comment on lines 28 to 36
public static func buildExpression(_ component: CGFloat) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(component)]
}
public static func buildExpression(_ component: Double) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(component)]
}
public static func buildExpression(_ component: Int) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(CGFloat(component))]
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three methods enable treating raw numbers as fixed positions in the build expression. So instead of 8.fixed you could just write 8.

Happy to delete these for explicit parity with existing APIs. Also happy to add this functionality to existing APIs if folk would like.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I agree if we support unspecified numbers they should be fixed, but I think it would be better to keep it explicit. I could see it being very confusing to open a file and see

applyVerticalSubviewDistribution {
    oneView
    1
    twoView
}

Not having this context, I'd probably assume that's a flexible spacer. For now I think probably best to keep it explicit.

Comment on lines 28 to 36
public static func buildExpression(_ component: CGFloat) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(component)]
}
public static func buildExpression(_ component: Double) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(component)]
}
public static func buildExpression(_ component: Int) -> [ViewDistributionSpecifying] {
return [ViewDistributionItem.fixed(CGFloat(component))]
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I agree if we support unspecified numbers they should be fixed, but I think it would be better to keep it explicit. I could see it being very confusing to open a file and see

applyVerticalSubviewDistribution {
    oneView
    1
    twoView
}

Not having this context, I'd probably assume that's a flexible spacer. For now I think probably best to keep it explicit.

@dfed dfed force-pushed the dfed--view-distribution-builder branch from c5ba8b5 to 143dafe Compare April 21, 2024 23:03
@dfed dfed marked this pull request as ready for review April 21, 2024 23:04
@dfed dfed requested a review from NickEntin April 21, 2024 23:04
@dfed
Copy link
Collaborator Author

dfed commented Apr 21, 2024

@NickEntin wrote result builder tests you might enjoy, and removed more of the duplicative tests. Ready for a proper review (and CI run)!

Copy link
Collaborator

@NickEntin NickEntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for adding this!

@NickEntin NickEntin merged commit e5e3185 into square:master Apr 23, 2024
6 checks passed
@dfed dfed deleted the dfed--view-distribution-builder branch April 23, 2024 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ResultBuilder-backed API for subview distribution
2 participants