Conversation
watt
force-pushed
the
awatt/remove-legacy-mode
branch
from
db9f656 to
688dd8a
Compare
| @@ -0,0 +1,2 @@ | |||
| /// A token reference type that can be used to group associated signpost logs using `OSSignpostID`. | |||
| final class SignpostToken {} | |||
There was a problem hiding this comment.
moved from CacheFactory.swift
watt
force-pushed
the
awatt/remove-legacy-mode
branch
from
688dd8a to
ebfca07
Compare
| import CoreGraphics | ||
|
|
||
| /// The implementation of an `ElementContent`. | ||
| protocol ContentStorage: LegacyContentStorage, CaffeinatedContentStorage { |
There was a problem hiding this comment.
Real nit but is it even worth keeping the caffeinated name in the protocol anymore? I guess it's helpful historically?
There was a problem hiding this comment.
I kinda like the flexibility of composition, but there's no technical reason it has to stay. For the sake of reducing churn I'm not going to remove it in this PR.
| layoutMode: LayoutMode | ||
| ) -> CGSize { | ||
| switch layoutMode { | ||
| case .legacy: |
There was a problem hiding this comment.
Likewise, should we strip out this enum? Even the stuff I was playing with lived under options.
There was a problem hiding this comment.
Hmm I think I could flatten it into a struct without breaking too much more stuff, sure.
| @@ -229,32 +226,15 @@ extension Flow { | |||
| cache: inout () | |||
There was a problem hiding this comment.
This isn't super specific to this PR but it didn't really seem like anyone used this cache parameter, do they? Should we get rid of that as well?
There was a problem hiding this comment.
We don't write many new layouts these days, and the existing ones were never really updated to take advantage of it. But like the most of the caffeinated API, it mimicks the SwiftUI Layout, so I don't really want to remove it.
maxg-square
left a comment
maxg-square
left a comment
There was a problem hiding this comment.
Couple nits but overall looks great, you love to see it:
2 participants
Removes the legacy layout mode! May be easier to review by commit.
Following this change, layouts that define a single trait type will be required to explicitly conform to
SingleTraitLayoutto use the existing API for accessing traits (previously this was implicit viaLegacyLayout).I've updated layouts to remove legacy implementations, and in some cases refactored to remove the "shim" that allowed a shared implementation. But not all of them; in particular I left the Stack layout as-is, so that we can test if there's any performance impact of eagerly reading traits into an array or if the array allocation itself outweighs that.
Alongside this removal I also removed the
fillParentmode ofConstrainedAspectRatio, which was already deprecated because it's not useful under caffeinated layout.BREAKING CHANGE: removes
LayoutMode.legacy,LegacyLayout,ConstrainedAspectRatio.ContentMode.fillParent, and requires layouts that define a trait type to add theSingleTraitLayoutconformance explicitly.