-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
Allow modified views to fill their parent if a child requires it #165
Conversation
Generated by 🚫 Danger Swift against 8ab706a |
@@ -79,6 +79,7 @@ private extension EdgeInsets { | |||
} | |||
|
|||
extension _PaddingLayout: DOMViewModifier { | |||
public var orderDependent: Bool { true } |
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.
nit
public var orderDependent: Bool { true } | |
public var isOrderDependent: Bool { true } |
@@ -19,6 +19,12 @@ public typealias ModifiedContent = TokamakCore.ModifiedContent | |||
|
|||
public protocol DOMViewModifier { | |||
var attributes: [String: String] { get } | |||
/// Can the modifier be flattened? | |||
var orderDependent: Bool { get } |
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.
var orderDependent: Bool { get } | |
var isOrderDependent: Bool { get } |
} | ||
|
||
extension DOMViewModifier { | ||
public var orderDependent: Bool { false } |
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.
public var orderDependent: Bool { false } | |
public var isOrderDependent: Bool { false } |
@@ -42,6 +48,7 @@ extension _ZIndexModifier: DOMViewModifier { | |||
} | |||
|
|||
extension _BackgroundModifier: DOMViewModifier where Background == Color { | |||
public var orderDependent: Bool { true } |
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.
public var orderDependent: Bool { true } | |
public var isOrderDependent: Bool { true } |
content | ||
}) | ||
if let adjacentModifier = content as? AnyModifiedContent, | ||
!(adjacentModifier.anyModifier.orderDependent || domModifier.orderDependent) { |
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.
!(adjacentModifier.anyModifier.orderDependent || domModifier.orderDependent) { | |
!(adjacentModifier.anyModifier.isOrderDependent || domModifier.isOrderDependent) { |
@@ -32,6 +32,7 @@ extension _OverlayModifier: DOMViewModifier where Overlay == _ShapeView<_Stroked | |||
|
|||
// TODO: Implement arbitrary clip paths with CSS `clip-path` | |||
extension _ClipEffect: DOMViewModifier { | |||
public var orderDependent: Bool { true } |
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.
public var orderDependent: Bool { true } | |
public var isOrderDependent: Bool { true } |
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 is great, just a nitpick to make boolean property naming consistent.
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.
Lovely! 👍
For instance, a
ModifiedContent
view wrapping aVStack
with aSpacer
will allow the stack to fill the parent's height by puttingheight: 100%;
on itself. This goes for all views that have a childSpacerContainer
that fills either axis.