Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow modified views to fill their parent if a child requires it #165

Merged
merged 2 commits into from
Jul 7, 2020

Conversation

carson-katri
Copy link
Member

For instance, a ModifiedContent view wrapping a VStack with a Spacer will allow the stack to fill the parent's height by putting height: 100%; on itself. This goes for all views that have a child SpacerContainer that fills either axis.

@carson-katri carson-katri added the bug Something isn't working label Jul 7, 2020
@carson-katri carson-katri requested a review from MaxDesiatov July 7, 2020 01:47
@ie-ahm-robox
Copy link
Collaborator

Fails
🚫

Sources/TokamakDOM/DOMRenderer.swift#L81 - Line should be 100 characters or less: currently 127 characters (line_length)

🚫

Sources/TokamakDOM/Shapes/Shape.swift#L21 - Line should be 100 characters or less: currently 125 characters (line_length)

Generated by 🚫 Danger Swift against 8ab706a

@@ -79,6 +79,7 @@ private extension EdgeInsets {
}

extension _PaddingLayout: DOMViewModifier {
public var orderDependent: Bool { true }
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit

Suggested change
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 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
var orderDependent: Bool { get }
var isOrderDependent: Bool { get }

}

extension DOMViewModifier {
public var orderDependent: Bool { false }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public var orderDependent: Bool { true }
public var isOrderDependent: Bool { true }

content
})
if let adjacentModifier = content as? AnyModifiedContent,
!(adjacentModifier.anyModifier.orderDependent || domModifier.orderDependent) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
!(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 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public var orderDependent: Bool { true }
public var isOrderDependent: Bool { true }

Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a 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.

@carson-katri carson-katri requested a review from MaxDesiatov July 7, 2020 14:18
Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Lovely! 👍

@carson-katri carson-katri merged commit 1f2203d into main Jul 7, 2020
@carson-katri carson-katri deleted the fill-parent-hotfix branch July 7, 2020 14:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants