-
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 InspectorBlockInfo using SlotFill method #66170
base: trunk
Are you sure you want to change the base?
Add InspectorBlockInfo using SlotFill method #66170
Conversation
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jarekmorawski. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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, @karthick-murugan! 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. |
See my comment #49887 (comment) left under the original issue:
The slot fill is located in: It likely makes sense to rename it to the name used in this PR |
Thanks for your hard work here @karthick-murugan and for the ping for review 👍
The slot fill was made private specifically to avoid concerns around the potential for such a prominent slot to be abused. @gziolo it looks like the comment immediately following yours on the original issue expressed the same concerns.
This PR is essentially a different take on #45437 that was abandoned for those reasons. Some more history and context can be found there and related issues.
There were some discussions in walkthroughs and demos around that time that expressed the same sentiment but I can't put my finger on links for those at the moment. If they are really needed, I can go on a deeper dig when I have the bandwidth. Without a plan to address, or a consensus to dismiss, those concerns, I'm not sure the proposal to make such a slot public should proceed. |
I disagree with these concerns because the same goal can be achieved with CSS styling which we saw overused by plugins and themes on live websites. The slot is a semantic place to add additional details for the block's description. Let's not complicate it too much and put more trust in the extenders. Historically, using the wrapping element was sufficient to promote the correct usage, so in this case, a wrapping
Again, the feedback from @jarekmorawski is valid. My point is that the link can still get styled as a button with icons using custom CSS. Not to mention that custom JavaScript is another option to tweak the default block editor's behavior. |
I appreciate the pragmatic thoughts @gziolo 👍
Additional details about the block, yes. We can already see with things like block transforms though, that there's an inclination to put non-description/details/info things in that area. This is in part why there was previous pushback on a slot here and a suggestion that we need a better design for these use cases. FWIW, I'm not outright against a slot here. After all, I had an earlier PR proposing to add one. It would be good to maybe have an alternate design to be able to weigh options before committing on an approach. |
I am typically the biggest champion of additional slots, but I am not so sure about this one. You can already add any content you want in the For what it's worth, I also don't think the Add new post on the Query block should even be there. Clicking it takes you completely out of the editing experience of the post. |
#67189 is underway to remove the ""Add new post" link from the Query Loop block. This means that the private It might be worth discussing whether this private |
The private slot fill was only added initially to maintain the pre-existing UI when block inspector tabs were introduced. I know there's been a lot of discussion in different channels around adding links to the block card or not. My understanding of the general consensus is that we need to find a better home for these links rather than lean into the slot fill. To me the end goal then will be to remove the private |
What?
Fixes 49887
This PR adds new Slot InspectorBlockInfo for extending the block's description
Why?
Since non-string descriptions for blocks are deprecated, we have added new slot for adding non-string values.
How?
Created a new slot for InspectorBlockInfo below the block description and it can be filled by custom blocks or default blocks using the Fill method.
Testing Instructions
Checkout this branch to your local
Import the SlotFill to any of the default or custom block edit.js file
import { InspectorBlockInfoFill } from '@wordpress/block-editor';
Note: If needed you can change the function component name. Use the same name while calling it in point 4.
<InspectorBlockInfo />
Before adding the InspectorBlockInfo in the backend
After adding the InspectorBlockInfo in the backend
Preview Block Screenshot which does not show the link as expected.
Testing Instructions for Keyboard
Screenshots or screencast