#253 | Enable and update Empty State views for Sayings & Categories #491
Conversation
| static func uniform(_ inset: CGFloat) -> UIEdgeInsets { | ||
| .init(top: inset, left: inset, bottom: inset, right: inset) | ||
| } | ||
|
|
||
| static func vertical(_ verticalInset: CGFloat) -> UIEdgeInsets { | ||
| .init(top: verticalInset, left: 0, bottom: verticalInset, right: 0) | ||
| } | ||
|
|
||
| static func horizontal(_ horizontalInset: CGFloat) -> UIEdgeInsets { | ||
| .init(top: 0, left: horizontalInset, bottom: 0, right: horizontalInset) | ||
| } |
There was a problem hiding this comment.
Probably not a blocker, but something to consider: horizontal and vertical threw me for a loop up above because I couldn't tell if the inset was being uniformly applied to each edge, or if the total inset was being divided among each edge along that axis. APIs like this one on CGRect are probably what make me overthink things like this, but alas.
Having them be static functions is not bad, but I'm almost wondering if we would get more call site clarity out of following the SwiftUI-ish pattern of having UIEdgeInsets(top: CGFloat = .zero, left: CGFloat = .zero, bottom: CGFloat = .zero, right: CGFloat = .zero)?
Also: I know they're existing code but the operators also threw me off, so maybe I'm just having a case of EOD.
| extension StringProtocol { | ||
| /// The range spanning the full length of the String | ||
| var fullRange: Range<Index> { | ||
| startIndex ..< endIndex | ||
| } | ||
|
|
||
| /// The range spanning the full length of the String, represented as an `NSRange` | ||
| var fullNSRange: NSRange { | ||
| nsRange(from: fullRange) | ||
| } | ||
|
|
||
| /// Converts a range of Indices to its `NSRange` representation. | ||
| /// | ||
| /// - Parameter rangeExpression: The range of string indices | ||
| /// - Returns: The `NSRange` representation of the `rangeExpression` | ||
| func nsRange<R: RangeExpression>(from rangeExpression: R) -> NSRange where R.Bound == Index { | ||
| NSRange(rangeExpression, in: self) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm definitely into the idea of having the range of indices available in a more convenient way, but if we're extending something as generalized as StringProtocol I would love to see something that feels more like it's part of Foundation if that makes sense? Something like indexRange or rangeOfIndices? Just typing out loud here 🤔
I'm not personally in love with StringProtocol being able to produce NSRange values, in part because it feels like we're overloading that type unnecessarily when NSRange gives us a really clear initializer... but also ns being lowercased just feels wrong. Especially when you consider line 19 where it reads like an NSRange initializer but in SpONgeBob case 😛
I know we want to shift more in the direction of attributed strings for text styling going forward, and I wonder if there's anything we can do to extend attributed string to be more amenable to Swift strings as opposed to shoving that responsibility downward into StringProtocol 🤔 A discussion for another day!
|
Test Notes 2/23:
|
|
@moyerr and @sgordon1024 Are we removing the pagination from the screen when it's in an empty state? The Figma designs show the pagination elements. I have not looked at the work on this PR branch, I am just going off of the screenshots shared in the description of this PR. I just want to make sure because I was actually just working on the existing pagination tests, and will make adjustments for this. Update: |
|
This is still looking good after the latest commit: 🚢 Noticed an UI issue with orientation but that is not related and will be captured as a separate bug. |
Closes #253
Description of Work
This PR had two main goals:
EmptyStateViewto better reflect what is captured in the designs.Screenshots