-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support editing custom posts #25208
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
base: trunk
Are you sure you want to change the base?
Support editing custom posts #25208
Conversation
Generated by 🚫 Danger |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
dcalhoun
left a comment
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 perceive the draft status and current code to mean this is not ready for formal review. Regardless, I left a few thoughts for consideration from an initial review and testing.
Overall, things seem to operate relatively well. I noted one preexisting bug: CMM-1214
To elaborate on my original request for an integration PR, I primarily wanted to ensure an integration would merge relatively quickly after we merge the GutenbergKit (GBK) PR to avoid breaking changes in the GBK library blocking other GBK work. Sorry if I did not communicate that clearly enough previously. Thank you for beginning this work nonetheless.
Modules/Package.swift
Outdated
| .package(url: "https://github.com/wordpress-mobile/NSURL-IDN", revision: "b34794c9a3f32312e1593d4a3d120572afa0d010"), | ||
| .package(url: "https://github.com/zendesk/support_sdk_ios", from: "8.0.3"), | ||
| .package(url: "https://github.com/wordpress-mobile/GutenbergKit", from: "0.13.1"), | ||
| .package(url: "https://github.com/wordpress-mobile/GutenbergKit", revision: "7a48d8756f33e3d25daa1fff5843d5cd6fdda887"), |
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.
When we are ready, we can integrate a GBK prerelease. We'll publish the prerelease via make release VERSION_TYPE=preminor DRY_RUN=true on GBK's trunk branch.
| let configuration = EditorConfigurationBuilder( | ||
| content: initialContent ?? "", | ||
| postType: "comment", | ||
| postType: .init(postType: "comment", restBase: "comments"), // FIXME: "comment" is not a post type. |
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.
If we feel compelled to "fix" the post type, we might also consider fixing the site URL and API root. GBK currently requires all of these, but effectively discards them when "offline mode" is enabled.
For context, this comment editor is currently unused.
WordPress/Classes/ViewRelated/NewGutenberg/PostGBKEditorViewController.swift
Outdated
Show resolved
Hide resolved
Absolutely. I will not merge the GBK PR until this PR is ready. |
6a6c440 to
33deb2f
Compare
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 30917 | |
| Version | PR #25208 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | b167166 | |
| Installation URL | 2qe2k2ljml9cg |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 30917 | |
| Version | PR #25208 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | b167166 | |
| Installation URL | 5mdv4bovbmu88 |
8e6668a to
8a4c6b0
Compare
dcalhoun
left a comment
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 changes tested well for me overall. I was able to create/edit/update/publish posts, pages, and "books" post types.
I left a few inline suggestions and questions worth considering.
Also, I note the CPT header lacks bottom padding matching the post/page header. Additionally, the CPT more menu does not support all the options of the post/page header, but this may already be a known limitation. See images below.
WordPress/Classes/ViewRelated/NewGutenberg/CustomPostEditorViewController.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/NewGutenberg/CustomPostEditorViewController.swift
Outdated
Show resolved
Hide resolved
| // TODO: Check if the post supports Gutenberg first? | ||
| CustomPostEditor(client: client, post: post, details: details, blog: blog) { | ||
| Task { | ||
| _ = try await service.posts().refreshPost(postId: post.id, endpointType: endpoint) | ||
| } | ||
| } | ||
| CustomPostEditor(client: client, post: post, details: details, blog: blog) |
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.
We briefly discussed this before (see p1769044197504499-slack-C08UN8YN6KW), but editing classic posts with GBK is less useful at the moment. How difficult might it be to open these classic CPT entries in Aztec? I'm guessing it may be difficult. Alternatively, should we detect them and disallow editing them and display a more helpful notice?
Custom post type editing only supports posts created with the block editor at this time. This post appears to contain incompatible 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.
Good call! That's related to the TODO in this code. Are there any reasonable ways to detect if the post is "classic"?
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 believe we've relied upon the existing containsGutenbergBlocks logic in the past. Unfortunately, I do not believe there is a more reliable indicator available.
|
@dcalhoun Thanks for the review. I have addressed all your comments. |
Yeah, we don't support preview and post settings yet. |
dcalhoun
left a comment
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.
Latest changes work well for me. Addressed my prior findings. Thanks!
It's likely worthwhile awaiting Jeremy's feedback.
WordPress/Classes/ViewRelated/NewGutenberg/CustomPostEditorViewController.swift
Outdated
Show resolved
Hide resolved
efda525 to
41807d8
Compare
|











Description
This PR expands editor cache management to all post types. With the integration of wordpress-mobile/GutenbergKit#299, we can now edit custom posts in the GBK editor.
Testing instructions
Verify the GBK editor continues to work with regular posts and pages.
Verify you can edit custom posts (available in self-hosted sites) using the GBK editor.