Custom Posts: Show published pages with hierarchy#25353
Custom Posts: Show published pages with hierarchy#25353crazytonyli wants to merge 5 commits intotrunkfrom
Conversation
Generated by 🚫 Danger |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 31404 | |
| Version | PR #25353 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | cc5e308 | |
| Installation URL | 7q5o7imqhuiv0 |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 31404 | |
| Version | PR #25353 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | cc5e308 | |
| Installation URL | 5t7h5mrqmq5t0 |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
kean
left a comment
kean
left a comment
There was a problem hiding this comment.
I've tested the changes on one of my sites and was not able to get the new screens to show the hierarchy. In addition, some of the pages appear to be missing.
| } | ||
| } else { | ||
| ForEach(Array(items.enumerated()), id: \.element.id) { index, item in | ||
| let showSubdirectoryIcon: Bool = { |
There was a problem hiding this comment.
(nit) I'd suggest extracting showSubdirectoryIcon and also extracting the two branches for flat/hierarchal data.
| /// built once from the complete dataset. | ||
| private func fetchAllPages() async { | ||
| isBatchSyncing = true | ||
| defer { isBatchSyncing = false } |
There was a problem hiding this comment.
The web version bails if there are too many pages. If a site has more than 100 pages, a hierarchal view would be unusable as it would require too much scrolling, so there is no point attempting to show the hierarchy as it would only make the user experience worse. For a site that has a large number of pages, this would also become a performance issue. wdyt about following how the web approaches it?
There was a problem hiding this comment.
wdyt about following how the web approaches it?
This PR reused the existing Pages screen display: fetching all pages and displaying the page hierarchy. And this only happens on the Published tab. Maybe we can defer the refactor to later if needed?
If a site has more than 100 pages, a hierarchal view would be unusable as it would require too much scrolling, so there is no point attempting to show the hierarchy as it would only make the user experience worse.
This is how it works in this PR:
- Show the cached pages if available. This will almost instantly show a hierarchy view.
- Start to fetch all the pages. At the moment, the pagination size is 20, which I will change to 100 later (pending Allow changing the
per_pageargument Automattic/wordpress-rs#1220). That means we should be able to make a few HTTP requests to fetch all pages. - While the pages are fetching in the background and new pages come in, the page list is not updated.
- After all pages are fetched, the page list is updated to show the hierarchy view.
I think this UX should be okay?
|
@kean The hierarchy view is only shown in the "Published" tab. I think it makes sense to show it in the "All" tab too, which I'll update. I'll look into why posts are missing from the "All" tab, though. That probably uncovers some other issues. |
|
I have created a Linear issue https://linear.app/a8c/issue/CMM-1941 to track the post missing issue. |
|
4 participants
Description
In the existing Pages screen, all pages are fetched and reorganized into a tree. This PR implements the same on the custom post list.
Testing instructions
Compare the existing Pages screen with the one backed by the wordpress-rs library. They should show the same list of pages.