Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @benazeer-ben! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
ramonjd
left a comment
There was a problem hiding this comment.
This is working really well, thank you!
I tested posts and pages in the site editor (published and non-published), and also pages in the site editor.
The command appears as expected with the correct label, depending on the post status. ✅
I think for the first iteration it might be safer to limit the postType to page or post for now.
What do you think?
|
Looks like this PR needs a quick rebase too. |
…o enhancement/command-palette-view-page
|
Hi @ramonjd Thanks for your review and suggestions.
Yes, right. As per the ticket definition it is mentioning support for pages. So this PR is mainly developed for pages and posts, we can consider the other postTypes later as you suggested. |
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
|
I think it just needs a quick rebase and we're good to go 👍🏻 |
| } ); | ||
| } | ||
| if ( isViewable ) { | ||
| if ( postType === 'post' || postType === 'page' ) { |
There was a problem hiding this comment.
I appreciate your work on this @benazeer-ben Thank you!
Sorry, this wasn't known to me before, but I think a command already exists to preview in the editor:
Here they are together:
It's been there for some time.
I think this PR's command adds a little sugar by checking post status and having a "view" link, but the existing command will work for all posts, plus it also saves before previewing using __unstableSaveForPreview.
So, with this knowledge, maybe we need to combine. One option would be to rather use the existing command, and add a generic View in a new tab if the post is published (so don't gate for pages or posts). I'm not sure.
cc @jameskoster @youknowriad (who added the existing command)
There was a problem hiding this comment.
Thanks for highlighting this! @ramonjd
This PR adds some useful refinement by incorporating checks for the post status and providing a "View" link specifically for published posts, which could serve as a valuable enhancement.
With this in mind, rather than combining the commands, would it make sense to focus solely on the 'View' functionality in this PR and omit the preview option? This could simplify the implementation while keeping the focus clear.
I’d love to hear thoughts from other reviewers on this approach as well.
There was a problem hiding this comment.
I'd welcome more feedback on this idea but perhaps there's room for both commands? 'Preview' could be about checking a draft, or unsaved changes to a published record. 'View' could be about viewing a published record ignoring any unsaved changes.
- Editing a draft: "Preview in a new tab"
- Editing a published record
- With no changes: "View in new tab"
- With changes: "View in new tab" and "Preview unsaved changes in new tab"
I'd also question whether the "in new tab" text needs to be visible, I think the icon already communicates that behavior. If visually hidden we might simplify the commands; "Preview $post_type".
There was a problem hiding this comment.
With this in mind, rather than combining the commands, would it make sense to focus solely on the 'View' functionality in this PR and omit the preview option?
I think that's a good compromise for the first iteration.
'Preview' could be about checking a draft, or unsaved changes to a published record. 'View' could be about viewing a published record ignoring any unsaved changes.
Does that imply that we'd gate the existing preview command with a getEditedPostAttribute( 'status' ) check (not published), similar to what @benazeer-ben has done in this PR?
Then the changes in PR could be reduced in scope to editing a published record as mentioned.
There was a problem hiding this comment.
Does that imply that we'd gate the existing preview command with a getEditedPostAttribute( 'status' ) check (not published), similar to what @benazeer-ben has done in this PR?
I think 'Preview' could also appear if the page is published and there are unsaved changes. In that case the command would read "Preview unsaved changes".
There was a problem hiding this comment.
If there are any finalized suggestions, we can proceed with implementing them.
|
Hi I think we can reduce the scope and then consider opening another issue for the existing preview command if the text needs to be updated for that command. On the other hand, is there a reason this is limited to posts and pages and not viewable custom post types? The PR also needs another rebase. |
|
Hi @benazeer-ben, sorry for the late reply. I think this PR is almost ready to ship, but could you rebase this PR on top of the trunk branch? Thank you! |
What?
Fixes #55806
Why?
Currently view page/post command is not available.
How?
Added command for viewing page/ post from command box.
Testing Instructions
Screenshots or screencast
view-page-post.mp4