Order List: Date grouping and sections #257
Conversation
…ted section name into friendly string.
| import Foundation | ||
|
|
||
| extension Date { | ||
| /// Returns a NSDate instance with only its Year / Month / Weekday / Day set. Removes the time! |
There was a problem hiding this comment.
This Date extension comes from WordPressShared. I was not sure I should add the whole WordPressShared pod just for a single function, but I can if that is preferred.
There was a problem hiding this comment.
@mindgraffiti the WPShared pod is already included in our podfile. So, feel free to use that date helper directly. 😀
There was a problem hiding this comment.
@mindgraffiti Would it be possible to:
- delete this method
- update the
podfileto include theWordPressSharedpod in the Storage framework (i.e.pod 'WordPressShared', '1.0.8') - then use the
WordPressSharedversion inOrder+CoreDataClass.swift
?
There was a problem hiding this comment.
Just now getting back to this. Thank you for the instructions! I haven't imported a pod to a subproject before.
|
|
||
| // MARK: - Private Helpers | ||
| fileprivate enum Sections: String { | ||
| case Months = "0" |
There was a problem hiding this comment.
Sections enum is duplicated from here into Order+Woo.swift. Making it public here did not make it accessible to Order+Woo. (Possibly because of ReadOnlyEntity?)
|
@jleandroperez @bummytime this is ready for review. Thank you for introducing me to keyPaths Jorge 🥇 |
| /// Returns a Section Identifier that can be sorted. Note that this string is not human readable, and | ||
| /// you should use the *descriptionForSectionIdentifier* method as well!. | ||
| /// | ||
| @objc func sectionIdentifier() -> String { |
There was a problem hiding this comment.
@mindgraffiti since this method is required by a ViewController (App Side!), would it be possible to move this method to the main app? (extension StorageOrder).
|
@mindgraffiti just a general UI question — I noticed the sections' heights are larger in this PR vs. the i6 designs. Was this intentional?
|
bummytime
left a comment
bummytime
left a comment
There was a problem hiding this comment.
Nice job on this @mindgraffiti — I loved seeing this finally implemented! 😄 I left a few questions for you and a separate UI comment. Let me know your thoughts.
| import Foundation | ||
|
|
||
| extension Date { | ||
| /// Returns a NSDate instance with only its Year / Month / Weekday / Day set. Removes the time! |
There was a problem hiding this comment.
@mindgraffiti Would it be possible to:
- delete this method
- update the
podfileto include theWordPressSharedpod in the Storage framework (i.e.pod 'WordPressShared', '1.0.8') - then use the
WordPressSharedversion inOrder+CoreDataClass.swift
?
| /// | ||
| @objc func sectionIdentifier() -> String { | ||
| // Normalize Dates: Time must not be considered. Just the raw dates | ||
| let fromDate = dateCreated?.normalizedDate() |
There was a problem hiding this comment.
See my comment above about using the WordPressShared version ☝️ . Let me know your thoughts.
| /// Returns a Section Identifier that can be sorted. Note that this string is not human readable, and | ||
| /// you should use the *descriptionForSectionIdentifier* method as well!. | ||
| /// | ||
| @objc func sectionIdentifier() -> String { |
This reverts commit 0e97586.
…the main project rename, refine, refactor
|
@bummytime yeah, the Orders List is following a non-standard section header height in the i6 design, iirc. And I think the code that set up the section height was erased at some point. I'll open a separate PR to investigate what happened there. |
|
|
given in Slack, I didn't mark it here...my bad. Thanks for working hard on this PR 
Closes #138.
Follows WordPress-iOS Notifications implementation of grouping sections by fuzzy dates. Note that this is consistent with Android's UI.