-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add: View button to the post card panel. #60705
base: trunk
Are you sure you want to change the base?
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. |
Size Change: +125 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
abc20f2
to
45b527d
Compare
label={ action.label } | ||
icon={ action.icon } | ||
isDestructive={ action.isDestructive } | ||
size="compact" |
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.
You'll need to update the size to small
to match the designs. With this change you don't need the css changes you made here, so you can remove them.
! [ | ||
TEMPLATE_POST_TYPE, | ||
TEMPLATE_PART_POST_TYPE, | ||
PATTERN_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.
wp_navigation
is missing.
isDestructive={ action.isDestructive } | ||
size="compact" | ||
onClick={ async () => { | ||
await action.callback( [ item ] ); |
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 don't need to await anything here.. We can just call the action.callback
.
@@ -100,6 +122,7 @@ export default function PostCardPanel( { className, actions } ) { | |||
> | |||
{ title ? decodeEntities( title ) : __( 'No Title' ) } | |||
</Text> | |||
<ActionTrigger item={ post } action={ viewPostAction } /> |
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 think the end goal is to unify the actions and handle them holistically. Since we don't do that yet and consumers can also pass other actions, I think it makes more sense to rename ActionTrigger
to ViewPostLink
or something similar for now.
Good eye! I don't recall the issue where it was discussed adding that link, but I do remember that top toolbar addition being an important one as far as prominence, so I wouldn't remove it in the top toolbar. The redundancy of also having it in the panel is a good callout, though; it definitely exists there to afford the presence when the same panel shows in the Data View context in the future. But perhaps we should adopt this pattern for the panel? I'll defer to @jameskoster. |
Overall an argument for a light-weight inclusion in the inspector panel. Could just link to the homepage if on a template. |
45b527d
to
7342f5a
Compare
Hi @jasmussen, @jameskoster, so just to confirm we stop the addition of the view button to the postcard panel (leaving it in ellipsis as it is now) and we add the top toolbar view button at the side of save in edit site to match what edit post already has. Is that right? |
Part of #59689.
Adds a view button to the PostCardPanel. It appears in both edit post and edit site. When clicked opens the post link in a new window.
Testing Instructions
Opened a page on the side editor and verified the view button works as expected.
Repeated the test for the post editor.
Testing Instructions for Keyboard