Skip to content
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

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

karthick-murugan
Copy link
Contributor

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

  1. Checkout this branch to your local

  2. Import the SlotFill to any of the default or custom block edit.js file

import { InspectorBlockInfoFill } from '@wordpress/block-editor';

  1. Add this functional component to the edit function of the block in the same edit.js file.
const InspectorBlockInfo = () => {
    return (
        <InspectorBlockInfoFill>
            <a href='https://wordpress.org/documentation/article/buttons-block/'>More Information</a>
        </InspectorBlockInfoFill>
    );
};

Note: If needed you can change the function component name. Use the same name while calling it in point 4.

  1. Call the functional component in the same edit.js file

<InspectorBlockInfo />

Before adding the InspectorBlockInfo in the backend
before_block_info_added

After adding the InspectorBlockInfo in the backend
after_block_info_added

Preview Block Screenshot which does not show the link as expected.
no_html_preview

Testing Instructions for Keyboard

Screenshots or screencast

Copy link

github-actions bot commented Oct 16, 2024

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.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
  • Labels found: [Feature] Block API, [Feature] Extensibility, First-time Contributor.

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.

Copy link

github-actions bot commented Oct 16, 2024

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 props-bot label.

Unlinked Accounts

The 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.

Unlinked contributors: jarekmorawski.

Co-authored-by: karthick-murugan <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: ndiego <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: mtias <[email protected]>
Co-authored-by: fabiankaegy <[email protected]>
Co-authored-by: simison <[email protected]>
Co-authored-by: jeherve <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 16, 2024
Copy link

👋 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.

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Feature] Extensibility The ability to extend blocks or the editing experience labels Oct 20, 2024
@gziolo gziolo self-requested a review October 20, 2024 05:23
@gziolo
Copy link
Member

gziolo commented Oct 21, 2024

See my comment #49887 (comment) left under the original issue:

There is a private slot fill that the Query block uses:

We should promote it to the public to resolve this issue.

The slot fill is located in:

https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/block-info-slot-fill/index.js

It likely makes sense to rename it to the name used in this PR InspectorBlockInfo and follow up with subsequent changes discussed in the issue.

@aaronrobertshaw
Copy link
Contributor

Thanks for your hard work here @karthick-murugan and for the ping for review 👍

There is a private slot fill that the Query block uses:

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.

There's a risk that placing a slotfill in such a prominent location in the Inspector may lead to abuse and I wonder if we'd put some guardrails in place?

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.

@gziolo
Copy link
Member

gziolo commented Oct 21, 2024

There is a private slot fill that the Query block uses:

The slot fill was made private specifically to avoid concerns around the potential for such a prominent slot to be abused.

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 <p> tag would be sufficient for the inner HTML to remain semantic and should not contain block elements.

There's a risk that placing a slotfill in such a prominent location in the Inspector may lead to abuse and I wonder if we'd put some guardrails in place? For instance, we'd only allow links (with or without icons), but not buttons or notices.

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.

@aaronrobertshaw
Copy link
Contributor

I appreciate the pragmatic thoughts @gziolo 👍

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.

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.

@ndiego
Copy link
Member

ndiego commented Oct 23, 2024

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 InspectorControls. The primary value I see to this slot is adding links as mentioned here, which I feel clutter the user interface.

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.

@t-hamano
Copy link
Contributor

#67189 is underway to remove the ""Add new post" link from the Query Loop block. This means that the private BlockInfo slot will no longer be used anywhere.

It might be worth discussing whether this private BlockInfo slot should be left.

@aaronrobertshaw
Copy link
Contributor

It might be worth discussing whether this private BlockInfo slot should be left.

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 BlockInfo SlotFill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Extensibility The ability to extend blocks or the editing experience First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block Registration: add SlotFill to extend a block's description
5 participants