Show post types on home screens#25233
Show post types on home screens#25233crazytonyli wants to merge 9 commits intoprototype-custom-post-types-localfrom
Conversation
Generated by 🚫 Danger |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 30922 | |
| Version | PR #25233 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 67d950e | |
| Installation URL | 0a0nc3tceiko8 |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 30922 | |
| Version | PR #25233 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 67d950e | |
| Installation URL | 7mcf1cc64nbe8 |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
| } | ||
|
|
||
| // TODO: | ||
| // - Log errors to sentry: https://github.com/wordpress-mobile/WordPress-iOS/pull/25157#discussion_r2785458461 |
There was a problem hiding this comment.
It's pretty easy to do with wpAssertionFailure and error included in user info. I did use these to monitor sync errors for posts.
| // filterBarContainer.backgroundColor = .systemGroupedBackground // .secondarySystemGroupedBackground | ||
| filterBarContainer.contentView.addSubview(filterBar) | ||
| filterBar.pinEdges(.top, to: filterBarContainer.safeAreaLayoutGuide, insets: UIEdgeInsets(.top, -filterBar.tabBarHeight)) | ||
| filterBar.pinEdges(.top, to: filterBarContainer.safeAreaLayoutGuide, insets: UIEdgeInsets(.top, -AdaptiveTabBar.tabBarHeight)) |
There was a problem hiding this comment.
It looks like it includes some unrelated changes. Does it need a rebase?
There was a problem hiding this comment.
This changes are related (see #25233 (comment)). I extracted the value to a constant, to be reused in the CPT list view.
| import WordPressData | ||
|
|
||
| @propertyWrapper | ||
| struct SiteStorage<Value: Codable>: DynamicProperty { |
There was a problem hiding this comment.
This is clever. Does it work as an observable property?
There was a problem hiding this comment.
I have added a SwiftUI preview to test this property wrapper. The preview code does not go through the .init(key:blog:) initialiser, because we don't have a blog id instance. But the core logic is tested.
| ) | ||
|
|
||
| if FeatureFlag.customPostTypes.enabled && blog.supportsCoreRESTAPI { | ||
| let pinned = SiteStorageAccess.pinnedPostTypes(for: TaggedManagedObjectID(blog)) |
There was a problem hiding this comment.
I took your suggestion here (p1770673465795309/1770627105.176769-slack-C04PWEZSYFL), maybe I misunderstood it?
I would probably simply inline these items in the top-level menu.
I didn't reuse the personalization because there is already a pin and unpin in the post types list. But I can look into unifying how they store values into UserDefaults.
There was a problem hiding this comment.
I should've been clearer in that message. The suggestion was to add them directly in the "Content" menu in the site details as opposed to displaying all CPTs under "More Content", which it now does. As for the Dashboard, I'd probably keep it opt-in (you can add CPTs there if you want). The "quick actions" menu is intentionally short, so it doesn't overshadow the cards below.
| import WordPressUI | ||
| import WordPressData | ||
|
|
||
| struct CustomPostTabView: View { |
There was a problem hiding this comment.
It's not clear if these changes are related to the PR, so I skipped reviewing them.
There was a problem hiding this comment.
Yes, these changes are intentionally included in this PR. I don't want to create another chained PR, considering the changes are relatively small (from the filter design to tab design).
There was a problem hiding this comment.
Gotcha, I'll review them as well tomorrow. Thank you.
|
|
||
| private let mapping: [String: String] = [ | ||
| // Commerce & Products | ||
| "products": "shippingbox", |
There was a problem hiding this comment.
I have looked into reusing Gutenberg icons and will share some screenshots later.
The only solution for matching wp-admin icons is using dashicons. As discussed in p1769760142081339-slack-C06S1QS55K9, I don't think we want to use them in the app.
At the moment, the CPT icons (set by plugin author) are mapped to SF symbols. We can expand the mapping as we go. But I imagine we don't have many choices if we map the icons to Gutenberg icons, which is a much smaller set than SF symbols.
Nonetheless, I think we can probably address this in a separate PR.
9704712 to
10bc524
Compare
Otherwise the user won't be able to unpin post types.
|









Note
This PR will be merged after #25208.
Description
There are two main changes:
cpt-list.mov