-
Notifications
You must be signed in to change notification settings - Fork 28
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
Rename "Element" to "Content" #150
Conversation
…ecomes 'ItemContent', and 'HeaderFooterElement' becomes 'HeaderFooterContent'.
68a90e1
to
332666a
Compare
* master: Rename none to notSelectable
@@ -1,31 +0,0 @@ | |||
// |
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.
Moved into mainline.
{ | ||
// | ||
// MARK: Creating Blueprint Element Representations | ||
// | ||
|
||
var element : BlueprintUI.Element { get } | ||
var elementRepresentation : Element { 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.
I changed this to line up with ProxyElement
, so if you make a ProxyElement
conform to BlueprintHeaderFooterContent
, you don't also need an element
property.
|
||
|
||
public struct ListItemElement : ItemElement | ||
public struct EmbeddedList : ItemContent |
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 name seemed a bit clearer, so went with this instead of ListItemContent
.
@@ -90,3 +80,19 @@ public struct ListItemElement : ItemElement | |||
ListView(frame: frame) | |||
} | |||
} | |||
|
|||
public extension EmbeddedList |
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.
Nested ListItemSizing
into EmbeddedList
.
@@ -209,8 +209,18 @@ public extension ItemElement where Self:Equatable | |||
} | |||
|
|||
|
|||
/// Provides a default implementation of `identifier` when self conforms to Swift's `Identifiable` protocol. | |||
@available(iOS 13.0, *) | |||
public extension ItemContent where Self:Identifiable |
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.
Added this as well for easier Identifiable
usage.
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 think #if swift(>=5.1)
is more correct here - Identifiable
should be available anywhere as long as it's Swift 5.1 right?
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.
Ahh yeah, maybe – let me try that; Xcode told me to add the iOS 13 check so I did
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, Xcode doesn't like it:
#if swift(>=5.1)
public extension ItemContent where Self:Identifiable
{
var identifier : Identifier<Self> {
.init(self.id)
}
}
#endif
Still asks me to do:
/Users/k/Desktop/Development/Listable3/Listable/Sources/Item/ItemContent.swift:214:41: 'Identifiable' is only available in iOS 13 or newer
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.
Dang, the docs didn't mention an iOS version so I thought it would work. Oh well!
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.
Might be because it was added in 5.1, which IIRC was the first ABI stable release, so it's now baked into iOS vs. embedded in the bin?
{ | ||
lhs.add(rhs) | ||
} | ||
|
||
public static func += <Content:ItemContent>(lhs : inout Section, rhs : Content) |
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.
These came in from BlueprintLists
.
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.
Might not be worth it, but could make migration easier if you added type aliases for public renamed types and deprecated them. Something like:
@available(*, deprecated, renamed: "ItemContent")
typealias ItemElement = ItemContent
@@ -209,8 +209,18 @@ public extension ItemElement where Self:Equatable | |||
} | |||
|
|||
|
|||
/// Provides a default implementation of `identifier` when self conforms to Swift's `Identifiable` protocol. | |||
@available(iOS 13.0, *) | |||
public extension ItemContent where Self:Identifiable |
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 think #if swift(>=5.1)
is more correct here - Identifiable
should be available anywhere as long as it's Swift 5.1 right?
I'd thought about doing this, but figured since we'll also be doing the bumping in SPOS, it wouldnt make a ton of diference – though I guess it could help people find the new APIs – thoughts? |
I know we're pre 1.0 but there's definitely consumers outside of SPOS already (i.e., that hacker news demo app, other side projects) so it'd be nice I think. |
Good point! Will do |
The naming of
ItemElement
,HeaderFooterElement
was a bit confusing, since it didn't really convey how it was different fromItem
orHeaderFooter
, and also made talking about Blueprint integration confusing, because Blueprint refers to its content asElement
. Thus, rename toItemContent
andHeaderFooterContent
, plus update all relevant code, docs, comments, etc.