-
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
Block type: add support link #11330
Block type: add support link #11330
Conversation
For me personally, it feels like plugins territory. I would love to hear feedback from others. |
I'm tempted to hear more use cases and examples of where this might make sense. Honestly it feels like links like these should be less "cookie cutter", and rather be worked into the prose of the paragraph, like a simple hyperlink. But could be I'm missing something? |
The description is text only at the moment, so it would be a raw link included... |
You mean it's supposed to be... 😅
@gziolo I don't follow you — wanna elaborate what you mean? |
I mean that they can use
I assumed it's text only, but it looks like we never validate it :) It makes only this whole process easier. /cc @youknowriad and @aduth - this is an interesting finding, it might mean that most of the fields we consider as string only might, in fact, accept components. |
I've encountered this too in the past. It's quite easy to overlook, unfortunately. gutenberg/packages/notices/src/store/actions.js Lines 44 to 47 in a1b1eec
It makes me conflicted because I think we're better to set the expectations earlier than to have it break at a point in the future, but the contract we establish makes it clear that this is to be a string, regardless of what other values incidentally happen to work. |
Perhaps also a good place for:
Why would block builder do that vs what we've done? Shouldn't |
Those use cases are totally fair, thanks for elaborating, and it especially makes sense in light of the textfield not allowing links (or it shouldn't). I'm not entirely sure a separate link below the box is the right solution though... maybe it is? It feels, to repeat myself, cookie-cutter: this is where you put text, this is where you put a link. It's great to provide APIs that are limited so as to ensure accessibility and usability even for plugins, but I'd love a sanity check on these thoughts. |
❤️ Anything that makes documentation more findable for users when they need it is a 👍 from me. Having an explicit mechanism for plugin developers to link to their documentation will ideally encourage developers to use it more. Ideally, this would be something we could expand in the future to provide inline help directly, rather than taking the user to a new site, making for a more seamless and uninterrupted flow. I'd be in favour of keeping the link separate from the description. This has a few benefits:
Regarding wording, I'd opt for "Help" or "How-to" or even just "Documentation" rather than "Support reference" here. (Added tag for copy review.) It would be great to get an accessibility perspective on this proposed change as well. (Added tag for accessibility feedback.) |
I second everything @sarahmonster said. I think "Help" or "Documentation" would be better than "Support Reference", and I think keeping it separate from the description would be best. |
It sounds like a consensus is emerging! |
"Support reference" and "documentation" both sound a little... serious? They're not normal-speech words for most people. "Help" feels like it doesn't give me quite enough context re: what happens when I click -- does that open a chat window, an email, a webpage? I'd love something that felt a little more relaxed but with a little more info, like "Setup details"/"Block details" if it needs to be short, or "How to use this block" or "Learn more about this block" if that can be accommodated. |
I like "Learn more about this block" as pointed out by @michelleweber, or even "Learn more" would work for me. I also like how it's displayed in the initial post... a link at the bottom of the description. My reasons are that it's right inline with what I'm reading, and that feels like the place I'd expect to see any help links if there were any. |
@simison - we are working on defining the server-side friendly definition of blocks, there is RFC opened which would be a great place to include your proposal given that it seems like there is agreement that it would be a nice addition. We will need to have this new property documented so it should be win-win if it's taken into account there from the start. |
{ | ||
blockType.support.external ? ( | ||
<ExternalLink href={ blockType.support.url }> | ||
{ blockType.support.label } |
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 prolly need to cast this as a string to be on the safe side.
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 prolly need to cast this as a string to be on the safe side.
Did you still intend to make this change?
There is also: There might be more documents which mention Block API :) At some point, we will have to include it in RFC, too. Ideally, the proposed structure can be defined inside a JSON file. |
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.
How can we move this forward? From the conversation here, it seems there still needs to be implemented support for the default label?
Is it considered blocked by the Block Registration RFC (#13693) ?
{ hasSupportLink && ( | ||
<div className="editor-block-inspector__card-support"> | ||
{ | ||
blockType.support.external ? ( |
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.
Can you explain what makes a support link "external"? Is it based on the origin/hostname of the URL, and if so, is that not something we should programmatically determine on behalf of the block developer, than to impose on them?
{ | ||
blockType.support.external ? ( | ||
<ExternalLink href={ blockType.support.url }> | ||
{ blockType.support.label } |
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 prolly need to cast this as a string to be on the safe side.
Did you still intend to make this change?
Trying to triage PRs today. It seems that this PR is stale. I'm going to close it for now. Thanks all for your efforts. @mtias I was also wondering what you think about this config? and whether it's worth considering refreshing this PR? |
I find that providing an inline support link right next to the block description could have a positive impact on the learning curve of new users. This would be particularly true in the case of blocks added by plugins, which can be considerably more complex than the ones in core. Could we open this PR just to revisit this? Thank you! |
Description
I've added a way to declare where block's support documentation lives, via a block type options.
The link would appear under block description either as an external or internal link.
During our block development, we've started adding links to support documentation directly in "description" option. It works for desktop visually but isn't obviously great declaratively since it's no more just "description" and also goes against Gutenberg design rules:
Would be great to have a declarative way to add support documentation and make it easier to show in an optimal way in other places such as mobile devices. Something like:
This is a quick prototype; if you think it's a good idea, I'm happy to look into what other work might be needed to adding such support.
Further ideas:
This could also support passing URL as a string:
Could support adding multiple links as an array
The label could be made optional, defaulting to something like "Support reference".
Instead of
support
, the option could also be calleddocumentation
.URL could be passed through
prependHTTP
How has this been tested?
WIP
Screenshots
Types of changes
Checklist: