Fix field with multiple constraints. #77
Conversation
| @@ -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() } | |||
There was a problem hiding this comment.
Do you think it could be possible to create a tuple that would let me use the constraintName along with the constraint itself? Something like constraintName.left?
There was a problem hiding this comment.
You mean like this:
indexedConstraint.field = field + ".\(constraint.attribute)"(outputs eg: "progressFill.right")
Or I was thinking I could add a property multipleConstraint: Bool and add the suffix in LiveUIApplier.swift.
Using tuple would maybe unnecessarily duplicate the field and attribute ... ?
There was a problem hiding this comment.
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.
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 ... ?
| @@ -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() } | |||
There was a problem hiding this comment.
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.
| 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.
I feel like this could be easily refactored into a more functional style.
|
|
||
| guard | ||
| constraints.count > 1, | ||
| let field = constraints.first?.field |
There was a problem hiding this comment.
Isn't this a potential problem? You got this field from every individual constraint before.
There was a problem hiding this comment.
I don't think so, here you can get only constraints belonging to the same constraintField, eg fillHorizontally returns two constraints, one leading and one trailing
Fix #76