-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix field with multiple constraints. #77
Closed
Closed
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,16 @@ public struct Constraint { | |
public static func constraints(name: String, attribute: XMLAttribute) throws -> [Constraint] { | ||
let layoutAttributes = try LayoutAttribute.deserialize(name) | ||
let tokens = Lexer.tokenize(input: attribute.text) | ||
return try layoutAttributes.flatMap { try ConstraintParser(tokens: tokens, layoutAttribute: $0).parse() } | ||
var constraints = try layoutAttributes.flatMap { try ConstraintParser(tokens: tokens, layoutAttribute: $0).parse() } | ||
if constraints.count > 1 { | ||
for (index, constraint) in constraints.enumerated() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this could be easily refactored into a more functional style. |
||
guard let field = constraint.field else { break } | ||
var indexedConstraint = constraint | ||
indexedConstraint.field = field + "\(index)" | ||
constraints[index] = indexedConstraint | ||
} | ||
} | ||
return constraints | ||
} | ||
|
||
func serialize() -> XMLSerializableAttribute { | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do you think it could be possible to create a
tuple
that would let me use theconstraintName
along with the constraint itself? Something likeconstraintName.left
?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.
You mean like this:
(outputs eg: "progressFill.right")
Or I was thinking I could add a property
multipleConstraint: Bool
and add the suffix inLiveUIApplier.swift
.Using tuple would maybe unnecessarily duplicate the
field
andattribute
... ?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'm not sure about the inner workings of ReactantUI in this area, but I'm thinking about creating a tuple so that when you want to reference a constraint in Swift code, you don't have to check whether you're working with right or left variant by using indices and using
progressFill.left
instead.Again, not sure if it's even possible if RUI is not ready for this kind of approach.
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.
Hmm, tuple solution would be great, but AFAICT it would not be very easy to do. Maybe first fix this bug and then create another issue marked as enhancement ... ?