-
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 descriptive text and a link to embed documentation in embed blocks #16101
Conversation
import { BlockIcon } from '@wordpress/block-editor'; | ||
|
||
const EmbedPlaceholder = ( props ) => { | ||
const { icon, label, value, onSubmit, onChange, cannotEmbed, fallback, tryAgain } = props; | ||
return ( | ||
<Placeholder icon={ <BlockIcon icon={ icon } showColors /> } label={ label } className="wp-block-embed"> | ||
<div className="components-placeholder__instructions"> | ||
{ __( 'Paste a link to the content you want to display on your site.' ) } |
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 you should be able to pass this text straight into the Placeholder
component as a prop instead of creating a separate div:
{ !! instructions && <div className="components-placeholder__instructions">{ instructions }</div> } |
Something like this would work:
<Placeholder
icon={ <BlockIcon icon={ icon } showColors /> }
label={ label }
className="wp-block-embed"
instructions={ __( 'Paste a link to the content you want to display on your site.' ) }
>
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.
Updated!
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.
This looks good! Nice improvement.
There's a unit test failing at the moment, it's a snapshot test that checks that the html of the Embed block is correct—it'll need to be updated to reflect the changes.
Not sure if that's something you've done before @melchoyce, happy to guide you through the process if needed (it's pretty easy) or I can push a commit myself that updates the tests.
Edit: I'll just leave some info :)
There's a guide here to updating snapshot tests:
https://developer.wordpress.org/block-editor/contributors/develop/testing-overview/#tldr-broken-snapshots
It says to run npm run test-unit -- --updateSnapshot --testPathPattern path/to/tests
in the terminal.
I personally do it differently (more steps but in my opinion it's a better feedback loop, and it will notify you of failed tests as you're developing):
- Run
npm run test-unit:watch
in the terminal - Make changes to the file and save the changes, e.g. as described in this comment Add descriptive text and a link to embed documentation in embed blocks #16101 (comment)
- The changes to the file should trigger a re-run of the tests.
- When they've finished running there will be a prompt like
1 snapshot failed from 1 test suite. Inspect your code changes or press 'u' to update them.
. If the changes look correct, press the 'u' key as it describes, and the snapshot will be updated. - Stage and commit the changed files (including the snapshot file).
Okay... I think that worked? |
Thank you, adding links to help is something we should be doing a lot more! Will review this more tomorrow if no one else has got to it before then, but it's looking great! |
Thanks Nikki!! Let me know if there's any other blocks offhand you think need some links 😄 |
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.
…rnmobile/open-video-by-browser-for-android * 'master' of https://github.com/WordPress/gutenberg: (34 commits) [RNMobile] Native mobile release v1.7.0 (#16156) Scripts: Document and simplify changing file entry and output of build scripts (#15982) Block library: Refactor Heading block to use class names for text align (#16035) Make calendar block resilient against editor module not being present (#16161) Bump plugin version to 5.9.1 Editor: Fix the issue where statics for deprecated components were not hoisted (#16152) Build Tooling: Use "full" `npm install` for Build Artifacts Travis task (#16166) Scripts: Fix naming conventions for function containing CLI keyword (#16091) Add native support for Inserter (Ported from gutenberg-mobile) (#16114) docs(components/higher-order/with-spoken-messages): fix issue in example code (#16144) docs(components/with-focus-return): fix typo in README.md (#16143) docs(block-editor/components/inspector-controls): fix image path in README.md (#16145) Add mention of on Figma to CONTRIBUTING.md (#16140) Bring greater consistency to placeholder text for media blocks. (#16135) Add descriptive text and a link to embed documentation in embed blocks (#16101) Update toolbar-text.png (#16102) Updating changelogs for the Gutenberg 5.9 packages release chore(release): publish [RNMobile] Fix pasting text on Post Title (#16116) Bump plugin version to 5.9.0 ... # Conflicts: # packages/block-library/src/video/video-player.android.js
Description
The lack of help text in the embed blocks has confused people. This PR adds some additional help text and a link to the embed docs in HelpHub.
Screenshots
Before:
After:
Fixes #8976.