Making WordPress.org

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#5303 closed task (blessed) (fixed)

Block plugin validation tool UI improvements

Reported by: tellyworth's profile tellyworth Owned by:
Milestone: Priority: normal
Component: Plugin Directory Keywords:
Cc:

Description

The UI for the forthcoming block validator tool is very rough and needs improvement.

Attachments (18)

validator-ui.diff (8.7 KB) - added by ryelle 4 years ago.
Initial UI improvements— some error message cleanup, restructured the flow of the results page, and removed the icon links for text links
github-branches.diff (966 bytes) - added by ryelle 4 years ago.
Allow passing in github branches, ex: https://github.com/ryelle/rmb-recipe-block/tree/try/block-json
single-parent-block.diff (1.1 KB) - added by ryelle 4 years ago.
Check that the plugin only exports a single parent-level block
style-plugin.diff (3.0 KB) - added by ryelle 4 years ago.
style-theme.diff (315.9 KB) - added by ryelle 4 years ago.
collapsible-details-plugin.diff (6.6 KB) - added by ryelle 4 years ago.
collapsible-details-theme.diff (317.4 KB) - added by ryelle 4 years ago.
Screen Shot 2020-07-08 at 6.37.34 PM.png (67.1 KB) - added by ryelle 4 years ago.
Screen Shot 2020-07-09 at 8.06.45 PM.png (117.9 KB) - added by JeffPaul 4 years ago.
block plugin validator
Screen Shot 2020-07-09 at 8.35.18 PM.png (173.8 KB) - added by JeffPaul 4 years ago.
block validator help
Screen Shot 2020-07-09 at 8.49.21 PM.png (43.7 KB) - added by JeffPaul 4 years ago.
Oof, looks like updates happened while I was documenting my previous testing feedback. In the latest revision, I see another issue likely related to my testing in Firefox. Namely that the expand/collapse actions are not obvious and I only stumbled onto them by clicking on a row in the WARNINGS section. There is no visual queue in Firefox that those rows are clickable and that the feedback for each warning row appears once you click on each row.
5303-self-service-add-remove-plugin.diff (2.9 KB) - added by tellyworth 4 years ago.
Allow plugin committers to add or remove their plugins from the block directory
5303-translations.diff (2.0 KB) - added by ryelle 4 years ago.
5303-try-add-zip-upload.diff (7.3 KB) - added by tellyworth 4 years ago.
An incomplete stab at adding support for zip file uploads
5303.diff (8.9 KB) - added by ryelle 3 years ago.
Update apiVersion to number, use rest_validate_value_from_schema to validate json object
Screenshot 2022-01-04 at 07.25.08.png (349.4 KB) - added by gziolo 3 years ago.
Invalid errors reported by Block Plugin Checker
namespaces.png (132.6 KB) - added by welcher 3 years ago.
Screenshot 2022-01-05 at 11.30.28.png (266.8 KB) - added by gziolo 3 years ago.
Block Plugin Checker fixed

Download all attachments as: .zip

Change History (113)

@ryelle
4 years ago

Initial UI improvements— some error message cleanup, restructured the flow of the results page, and removed the icon links for text links

#1 @tellyworth
4 years ago

In 10003:

Block directory: initial UI improvements for block validator.

Props ryelle.
See #5303.

#2 @tellyworth
4 years ago

In 10006:

Plugin directory: link validator messages to sample help page.

The docs probably belong somewhere else, but this will serve as an example.

See #5303

#3 @tellyworth
4 years ago

In 10007:

Plugin directory: make it easier to validate block plugins by name or URL.

This improves the input handler to accept either a plugin slug, or a URL like https://wordpress.org/plugins/the-slug/.

See #5303.

#4 @tellyworth
4 years ago

In 10011:

Plugin directory: add a pass/fail message to the block validator.

See #5303

@ryelle
4 years ago

Check that the plugin only exports a single parent-level block

#5 follow-up: @ryelle
4 years ago

single-parent-block.diff This check is just an idea, I don't think we've officially made it a rule — there should be only one "top level" block per block-plugin. Having a single block ID'd as the "main block" would be helpful for the block dir API too, make sure we're showing the right block name.

Currently this check only works with block.json files, since we can't/don't get the rest of the config settings from the JS or PHP calls. So for now, it's just a warning, and it could be incorrect.

@ryelle
4 years ago

@ryelle
4 years ago

#6 @ryelle
4 years ago

style-plugin.diff & style-theme.diff are a pair, some markup & CSS fixes to clean up the page a bit. I'm not sure I made the change in the right place in the theme, so I can move that around if needed.

#7 in reply to: ↑ 5 @tellyworth
4 years ago

Replying to ryelle:

single-parent-block.diff This check is just an idea, I don't think we've officially made it a rule — there should be only one "top level" block per block-plugin. Having a single block ID'd as the "main block" would be helpful for the block dir API too, make sure we're showing the right block name.

Yep this is great. At the moment there's a much lazier check which blindly assumes that < 5 blocks might be parent/child blocks but 5+ is too many. It definitely should be a rule that there's only one top level block. I did have an earlier idea that we could enforce only one block.json file per plugin, with that file representing the top-level block. But this is much more comprehensive. It seems reasonable to say "if you want to include multiple parent/child blocks, you need to use block.json files for all of them with the correct parent values."

Let's put this in, and update the 5 block rule in check_for_blocks to be a higher number. At some point if there are, I dunno, 15 or 20 blocks in there, even if the parent/child relationship is configured correctly it might be a sign that something's gone wrong.

This ticket was mentioned in Slack in #core-editor by ryelle. View the logs.


4 years ago

#9 @ryelle
4 years ago

In 10035:

Plugin Directory: Fix PHP warnings in block validator

See #5303

#10 @ryelle
4 years ago

In 10037:

Plugin Directory: Add github branch support to the block validator

See #5303

#11 @ryelle
4 years ago

In 10038:

Plugin Directory: Clean up block validation form & result messages.

See #5303

#12 @ryelle
4 years ago

In 10039:

Plugin Directory: First pass of style updates for block validator

  • Fix label alignment
  • Fix URL input alignment on smaller screens
  • Fix font sizes of messages in notices

Bonus plugin upload fix: add focus style to the file upload button

See #5303

#13 @ryelle
4 years ago

In 10040:

Plugin Directory: Add a check for one top-level block.

Multiple blocks are allowed, but there should be only one "main" block. This checks the block.json file for each block (if exists), and flags if there is more than 1 block without a parent property. This also fixes the check_for_blocks check to warn if more than 15 blocks are found.

See #5303

#14 @Ipstenu
4 years ago

I think the change in r10040 (checking for one parent) may be too aggro. For example, listicles gets flagged:

More than one top level block was found: Listicles, Listicle Item Title, List Item, List Item Content More about this.

But the reason is there are multiple children (grandchildren)

  • Listicles - parent block
  • List Item - individual item, child of Listicle
  • List title & List Content -- children of List Item, grandchildren of Listicles

Is there a way to account for that? I'm not the only person who uses that methodology :)

#15 @ryelle
4 years ago

If you add a block.json for each block, and specify that parent relationship it should understand that — so List Item should have a property parent: [listicle]; list title & list content should have parent: [list item] (using the block names).

We can't get that info out of the JS or PHP files, unfortunately.

#16 @ryelle
4 years ago

collapsible-details-plugin.diff & collapsible-details-theme.diff switch away from the external help page, and add inline expandable help text to each check type. I think this gives a better dev experience, and will be translatable. The downside is needing a code change to update docs, but it seems that's how the other pages work.

There are still todo notes & other questions in this patch, but it's a first pass at adding descriptions to each check. I also collapsed all the check_block_json_is_valid issues into one section, since it can get wordy :)

Speaking of check_block_json_is_valid, I think it's a little aggressive — my test block uses script handles for editorScript, and it's flagging every one of them since they don't end in .js. Is that a requirement for finding the assets in the API?

I also had the idea to make a check for a well-formed block name— it should be alphanumeric, start with a letter, and contain one slash for namespace. If we agree that this should be the plugin slug, it could also check that. Otherwise a basic check that it isn't one of [ cbg, example, block, core ] seems good to me (other example prefixes I should add?).

#17 @Ipstenu
4 years ago

If you add a block.json for each block, and specify that parent relationship it should understand that — so List Item should have a property parent: [listicle]; list title & list content should have parent: [list item] (using the block names).

Yeah that's exactly what I did.

  • listitem.js has parent: [ 'lez-library/listicles' ],
  • listdt.js has parent: [ 'lez-library/listitem' ],

So all it's doing is saying "Hey, you have multiple items picked as parents" which ... yes? Could it not check if the item it picked as parent _also_ has a parent?

If not, it's no big, but we should have a note in the docs that says "grandchildren items may cause a false positive"

#18 @ryelle
4 years ago

Hmmmm, it should only be counting blocks that don't have parent set. And you've got these properties set in the block.json files? What plugin URL are you using to check this?

#19 follow-up: @Ipstenu
4 years ago

Ah. I have the properties in the JS but NOT the block.json (we really need a better example block.json because right now it's going to drive people bonkers trying to figure anything out -- it needs to be like the readme.txt example)

I was checking https://plugins.svn.wordpress.org/listicles/trunk and NOW with whatever changes just went through it stopped telling me I had too many parents :) (all families are valid, Block checker!)

Which reminds me...

https://wordpress.org/plugins/developers/block-plugin-validator/help/#check_block_tag should clearly explain "Your plugin needs the tag block" people had been using blocks (plural) because ... who knows.

#20 @ryelle
4 years ago

we really need a better example block.json because right now it's going to drive people bonkers trying to figure anything out

I agree, and I think it's finally stable enough that we can do that :) I added an example & more info to these docs, which we can link to from the error messages.

#21 in reply to: ↑ 19 @tellyworth
4 years ago

Replying to Ipstenu:

Ah. I have the properties in the JS but NOT the block.json (we really need a better example block.json because right now it's going to drive people bonkers trying to figure anything out -- it needs to be like the readme.txt example)

This is a great point. Where does the sample block.json belong?

people had been using blocks (plural) because ... who knows.

The plural blocks tag makes sense for plugins that contain multiple blocks. I added a check for the singular tag more as a suggestion.

#22 @tellyworth
4 years ago

In 10048:

Plugin directory: styles for collapsible details in block validator.

Props ryelle.
See #5303.

#23 @tellyworth
4 years ago

In 10049:

Plugin directory: show expandable details for validator messages.

Props ryelle.
See #5303.

@JeffPaul
4 years ago

block plugin validator

@JeffPaul
4 years ago

block validator help

#24 @JeffPaul
4 years ago

I've been testing the Block Plugin Validator the last two days with our Block for Apple Maps plugin and wanted to share the following feedback (note that I'm testing on Firefox 78.0.1.):

NICE TO HAVE

  1. Widen the text field for "Plugin repo URL" so the entire helper text displays.
  2. Link to plugin source for the specific revision (in the case of my sample plugin that would be to https://plugins.trac.wordpress.org/browser/maps-block-apple?rev=2338279) as this will allow inspection of the plugin code in comparison to the block validator results.

DESIRED

  1. I see SUCCESS and a nice green-highlighted message so it looks like I'm all set, except the copy makes it sound like there are additional steps I need to take ("No problems were found. Your plugin has passed the first step towards being included in the Block Directory."). Only its not clear what those steps are. Having details on what happens next is helpful to understand if there's more I need to do or if its now sitting with some other team (PRT perhaps?).
  2. There are WARNINGS and NOTES sections each with items called out with "More about this." links that in some cases don't hyper/anchor link to anything in particular. If there's help needed on copy for those links I'm happy to help draft those, but having something that is linked to for those would be helpful.
  3. Its unclear if the WARNINGS are things I need to resolve in order for our block to appear in the Block Directory, but I'd assume things are ok as there are no "ERRORS" which would tell me I still have things to fix. If the WARNINGS need to be resolved, it would be helpful to know.
  4. On the Block Validator Help page, the "Block scripts" section has copy that stops mid-sentence. Getting that clarified will be helpful.
  5. In trying to find more details on what is needed for our block plugin to appear in the Block Directory I've searched for and found several different markdown pages on GitHub (Share your Block with the World, Block Type Registration RFC, and Block Registration) each of which provide information that would ideally live in the Plugin Handbook as the canonical source of information where developers go to learn about crafting their plugins (in this case a block plugin). I suspect that for most plugin developers that it is not immediately obvious to find the right markdown file in the right branch in the Gutenberg GitHub repo to figure out what's needed for their block plugin to appear in the Block Directory.
Last edited 4 years ago by JeffPaul (previous) (diff)

@JeffPaul
4 years ago

Oof, looks like updates happened while I was documenting my previous testing feedback. In the latest revision, I see another issue likely related to my testing in Firefox. Namely that the expand/collapse actions are not obvious and I only stumbled onto them by clicking on a row in the WARNINGS section. There is no visual queue in Firefox that those rows are clickable and that the feedback for each warning row appears once you click on each row.

@tellyworth
4 years ago

Allow plugin committers to add or remove their plugins from the block directory

#25 @tellyworth
4 years ago

In 10051:

Plugin directory: add self-service support to the block plugin validator.

This lets plugin committers add or remove their plugins from the Block Directory.

See #5303.

#26 @tellyworth
4 years ago

In 10052:

Plugin directory: change button text from Validate to Check

See #5303

#27 @tellyworth
4 years ago

In 10053:

Plugin directory: send an email when plugin is added to the block directory.

See #5303

#28 @tellyworth
4 years ago

In 10054:

Plugin directory: queue an import when adding plugins to the block directory.

See #5303

#29 @dd32
4 years ago

In 10055:

Plugin directory: When queueing a plugin for import after being added to the Block Directory, pass the tags_touched property as an array.

See [10054].
See #5303.

#30 @ryelle
4 years ago

In 10057:

Plugin Directory: Improve block checker management section

Add a link to plugin code for committers, and change button style to use primary for "Add" buttons.

See #5303

#31 @ryelle
4 years ago

In 10058:

Plugin Directory: Update block.json file checks in block checker

Remove the json file check, since the same items are checked for in check_block_json_is_valid. Improve messages in check_for_block_script_files.

See #5303

#32 @ryelle
4 years ago

In 10059:

Plugin Directory: Skip info notices when gathering check_block_json_is_valid warnings.

See #5303

#33 @ryelle
4 years ago

@jeffpaul thanks for running through this! as you've noticed, most of the copy is still in progress. I added a link to the plugin code in r10057. It should always run the checker on trunk, but if you think linking to the exact rev would be helpful I can add that in. The "more about this" links have been removed in favor of the inline comments.

We definitely need better guidance for "what's next" if there are errors & warnings (only errors prevent adding to the directory). Where would you want to see that info?

#34 @tellyworth
4 years ago

That's odd about the lack of visual cue in FireFox @JeffPaul. I did notice the lack of triangle icons in FF, and thought I'd fixed that with the summary style addition in r10048. Perhaps I didn't build the CSS correctly when I committed that - can confirm that the triangles are missing again.

#35 @tellyworth
4 years ago

In 10062:

Plugin dir theme: bust cache for r10048.

See #5303.

#36 @tellyworth
4 years ago

I forgot to bust the cache, that's why. Triangles should appear now on expandable items in Firefox. It still could use a better visual cue, but that's something at least.

#37 @JeffPaul
4 years ago

I can confirm that Firefox mobile Version 27.0 (18428) correctly shows the triangle icons.

#38 @ryelle
4 years ago

In 10065:

Plugin Directory: Update duplicate block name check

Switches the check to flag an error if we find the block name in a different repo plugin, or a warning if the found block has the same author.

See #5303

#39 @ryelle
4 years ago

In 10066:

Plugin Directory: Add a check for block name standards

This will flag as errors any block names that would not register in the editor (using the same regex). It also checks for some common block starter content namespaces, to ensure names are unique across the repo (as warnings).

See #5303

#40 @annezazu
4 years ago

Came here to provide feedback about the language and general experience of the block validator. JeffPaul pointed out much of what was on my mind above though! In any case, here's what's on my mind as general enhancements:

  • It would be great to have more links to documentation or places to get help. Right now, the experience can feel a bit like a dead end otherwise.
  • Put priority on any "warnings" found so they show above any success message. It's odd to me how often there's both a message with warnings yet a "success" message. It's not clear to me if it's actually okay to ignore the warnings (more on this later)
  • Make the distinction between separate warnings clearer. Right now, it's a bit confusing that some of the warnings have an arrow to read more and others.
  • When reviewing block plugins with fatal errors, warnings, and the "problems were encountered" language it becomes really hard to know what to focus on. I'd recommend changing the visual of "problems were encountered" to make it more distinct from Fatal Errors. Right now, it looks like I need to resolve all three sections to proceed (warnings, problems were encountered, fatal errors) when in actuality I just need to focus in on the fatal errors to start.
  • I might consider renaming "Warnings" to "Recommended Improvements" and "Fatal Errors" to "Blockers". This depends on whether warnings are actually blockers though!

This tool will help determine if a WordPress block plugin is suitable for inclusion in the Block Directory. Enter a SVN repository URL, Github URL, or plugin slug to check. Please note this is a work in progress.

To call out a specific example, it would be great to link to some wider context from the start. Perhaps even linking to this recent post for now would be helpful:

https://make.wordpress.org/plugins/2020/07/11/you-can-now-add-your-own-plugins-to-the-block-directory/

#41 @ryelle
4 years ago

Here's an attempt at moving stuff around & adding more context: https://cloudup.com/cKNeb_LcIQj (nothing's committed yet, so it's just some result screenshots)

  • Moved the overall result to the top of the "Result" section, so it's a little more obvious this is the outcome, and the rest are individual issues
  • Tweaked the language on the "success" message since there might be warnings, and switched it to info-style if the plugin is already in the block directory.
  • Renamed Warnings to "Recommendations"
  • Added descriptions to error & recommendation sections

The pattern of having the individual check results use the error/warning/info notice style could also be updated if that seems overwhelming, thoughts?

Make the distinction between separate warnings clearer.

Do you have any suggestions? I thought about having all these details default to open, but I don't know if that would be too much information.

It would be great to have more links to documentation or places to get help.

I think these efforts are scattered & in-progress right now, but as they come together I agree, we'll want a better intro here.

#42 @annezazu
4 years ago

This looks a ton better already! Thanks for digging right in. Much easier to understand what actions need to be taken. Some additional feedback...

  • In this screenshot https://cloudup.com/iSeHAh8aemA The language of "Your plugin has passed the first step towards being included in the block directory" is a bit confusing as my brain immediately goes to "What's the next step?". Without the UI having corresponding "step" related direction, it can feel like one's missing something. I might change this to something like "Your plugin has passed the checker tests and is ready to be included in the block directory."
  • In this screenshot https://cloudup.com/iXgZw_lNR12 I might change the language from "They need to be addressed before your plugin will work in the Block Directory" to "They need to be addressed in order for your plugin to be included in the Block Directory" to make it clearer that the plugin cannot be added since the checker is meant to be part of the flow for adding to the directory.
  • The language around recommendations can be a bit clearer. Perhaps "These are suggestions to improve your block. While they are not required for your block plugin to be added to the Block Directory, addressing them will help people discover and use your block."

Do you have any suggestions? I thought about having all these details default to open, but I don't know if that would be too much information.

I think what might help is that if each warning has more information so there is consistency in every warning having the arrow in front of it. That weirdly makes it easier to distinguish and creates a nice visual pattern. Agreed on default to open being too much.

This ticket was mentioned in Slack in #core-editor by annezazu. View the logs.


4 years ago

#44 @ryelle
4 years ago

In this screenshot https://cloudup.com/iSeHAh8aemA The language of "Your plugin has passed the first step towards being included in the block directory" is a bit confusing as my brain immediately goes to "What's the next step?".

The section right under that notice is the next step— the "Add … to Block Directory" button (block authors will see this section as "Committer Tools", without the edit link). How do you think we can make that more clear?

#45 @annezazu
4 years ago

I think that button is quite clear. I just think the language around "steps" is going to cause confusion. Usually when there's mention of steps in a flow there's a nice list or a clear continuation of language (ie "Your final step is to.."). Without it, I just think that part might be confusing. This is a bit nitpicky ;) but clarity doesn't hurt.

#46 @ryelle
4 years ago

In 10071:

Plugin Directory: Remove *.asset.php check from block checker

This check only applies to a specific build process, and having more than one asset file is not inherently a bad thing.

See #5303

#47 @ryelle
4 years ago

More updates: I removed most of the "next step" language entirely, so that it's more clear that the actual next step is to click the button (and added a message to upload the plugin to wp.org if it's a github scan); added descriptions to all the errors. see screenshots here.

#48 @JeffPaul
4 years ago

@webfactory reported on the make/plugins post:

Block Plugin Checker has a broken link. When a check fails due to “No blocks found in plugin.” it links to https://wordpress.org/plugins/developers/block-plugin-validator/TUTORIAL which redirects to https://wordpress.org/plugins/search/developers/ which is probably not intended to happen

I think the intended link was https://wordpress.org/plugins/developers/block-plugin-validator/help/

If I test the checker for https://plugins.svn.wordpress.org/autoshare-for-twitter, in the "Fatal Errors:" section I see "No blocks found in plugin." and "Learn how to create a block." that links to https://wordpress.org/plugins/developers/block-plugin-validator/TUTORIAL/. I'm not certain that I know of an exact link for a tutorial at this point, but the make/plugins post is the closest I know of for now so maybe we use that link instead?

#50 @ryelle
4 years ago

In 10078:

Plugin Directory: Improve feedback on block checker page

This restructures the page so the flow is more clear, adds clarity to what counts as a blocker, and adds more details to all flagged issues.

See #5303

#51 @tellyworth
4 years ago

In 10079:

Plugin dir: refactor asset discovery code to be reusable.

Needed for block checker; see #5303.

#52 @tellyworth
4 years ago

In 10085:

Plugin directory: better asset detection in block checker.

This should make the block checker use identical guesswork to the importer when looking for block assets.

See #5303.

#53 @tellyworth
4 years ago

In 10088:

Plugin directory: add some helper functions to the block validator class.

See #5303.

#54 @tellyworth
4 years ago

In 10089:

Plugin directory: catch known problem PHP function calls in block validator.

Some PHP calls are likely indicators of a plugin that is not compatible with the Block Directory. This will produce a warning or error if they are found in a plugin.

See #5303.

#55 @ryelle
4 years ago

In 10090:

Plugin Directory: Add wp_add_inline_script to warning function list in block checker

See #5303

#56 @ryelle
4 years ago

I've uploaded a patch (5303-translations.diff) that checks for wp_set_script_translations & warns if it's not found. It's not a block directory requirement, but I've come across a bunch of block directory plugins that have translations but don't add it— so the translations don't work. I'm just not sure if the block checker is the right place for a check like this, or if we should stick to improving documentation.

#57 @ryelle
4 years ago

r10101 & r10102 should be attached to this ticket (my mistake in the commits).

r10102 commits the 5303-translations.diff patch.

#58 @tellyworth
4 years ago

In 10104:

Plugin dir: catch PHP syntax errors when checking block plugins.

See #5303.

@tellyworth
4 years ago

An incomplete stab at adding support for zip file uploads

#59 @ryelle
4 years ago

In 10109:

Plugin Directory: Skip composer framework files when checking size of block plugins

See #5303

#60 @ryelle
4 years ago

In 10110:

Plugin Directory: Fix spelling & grammar issues on block checker

See #5303

#61 follow-up: @JeffPaul
4 years ago

I caught this question in the #core-coding-standards channel and wasn’t sure if this was an odd case we should consider with the checker?
https://wordpress.slack.com/archives/C5VCTJGH3/p1596239699173000

I just ran my plugin through the new Block Checker and it's giving a Fatal Error with this reason: Found PHP call header(). This is likely to prevent your plugin from working as expected. It's referring to this code at the top of the plugin:

if (!defined('ABSPATH')) {

header('Status: 403 Forbidden');
header('HTTP/1.1 403 Forbidden');
exit();

}

It was in my understanding that this is always a good thing to put at the top of a plugin or theme in order to avoid direct access. Why would this be considered a Fatal Error ?

#62 in reply to: ↑ 61 @dd32
4 years ago

Replying to JeffPaul:

I caught this question in the #core-coding-standards channel and wasn’t sure if this was an odd case we should consider with the checker?

Just noting that this was handled in #meta https://wordpress.slack.com/archives/C02QB8GMM/p1596240707221700

The suggestion was to not use the header() calls but to keep the exit() if wanted.
Allowing header() to be used for other reasons would likely signify that a plugin really wasn't a Block Plugin so it really seems like banning it's use entirely is probably best.

#63 @ryelle
4 years ago

In 10111:

Plugin Directory: Add a total size check for block plugins

This will trigger a warning if the total size of the plugin exceeds 1MB. A larger plugin will cause a longer wait when inserting into the editor, which degrades the user experience.

See #5303

#64 @tellyworth
4 years ago

In 10118:

Plugin directory: check block plugins for multiple namespaces.

See #5303.

#65 @ryelle
4 years ago

In 10128:

Plugin Directory: Fix textdomain on block namespace check

See #5303

#66 @tellyworth
4 years ago

In 10171:

Plugin dir: catch more multi-block plugins.

This makes the block plugin checker more strict about plugins that contain many top-level blocks, which won't work in the block directory.

See #5303.

#67 @tellyworth
4 years ago

In 10172:

Block dir: make invalid namespaces an error.

See #5303.

#68 @tellyworth
4 years ago

In 10173:

Plugin dir: check for unique namespaces in the block checker.

This should permit block namespaces to be reused by the same author, but otherwise require that the namespace be unique.

See #5303.

#69 @tellyworth
4 years ago

In 10174:

Plugin dir: block plugins shouldn't register shortcodes.

See #5303.

#70 @tellyworth
4 years ago

In 10176:

Plugin dir: improve block checker handling of json errors.

Links to the json file when displaying an error.

See #5303.

#71 @tellyworth
4 years ago

In 10180:

Plugin dir: support zip uploads in block validator.

This also refactors the code to be a little less messy.

See #5303.

#72 @tellyworth
4 years ago

In 10181:

Plugin dir: add test button for plugin reviewers.

See #5303.

#73 @tellyworth
4 years ago

In 10182:

Plugin dir theme: update block checker styles.

See #5303.

#74 @tellyworth
4 years ago

In 10189:

Plugin dir: prevent false trigger of the Test button.

See #5303.

#75 @tellyworth
4 years ago

In 10203:

Plugin dir: only show test results when available.

See #5303.

#76 @tellyworth
4 years ago

In 10204:

Plugin dir: fix the Add/Remove buttons.

The Test button was interfering with admin users.

See #5303.

#77 @tellyworth
4 years ago

In 10210:

Plugin dir: filter out broken blocks from block search API.

See #5303.

#78 @tellyworth
4 years ago

In 10212:

Plugin dir: revert r10210.

Seems like it broke block search.

See #5303.

#79 @tellyworth
4 years ago

In 10213:

Plugin dir: add an email to alert block developers about test failures.

See #5303.

#80 @tobifjellner
4 years ago

@tellyworth typo in https://meta.trac.wordpress.org/changeset/10213
...that attempts to insert the block %3%s into a post...

#81 @dd32
4 years ago

In 10225:

Plugin Directory: Block Validator: Fix a string typo.

Props tobifjellner.
See #5303.

#82 @JeffPaul
3 years ago

TLDR: Can someone help check and see if there's a fix needed for the Block Plugin Checker for the apiVersion field?

Details:
Note that @richtabor may have found a logic error on the apiVersion check on the Block Plugin Checker as its throwing a block.json file error for his Markdown Comment Block plugin that the apiVersion property must contain a string value. However the recent Block API Enhancements post seems to show this as an integer. I'm guessing this was a code copy mistake mimicking the same as the other version (string|false|null) check.

#83 @gziolo
3 years ago

I opened PR against the Gutenberg repository that clarifies that apiVersion in block.json should be a number:

https://github.com/WordPress/gutenberg/pull/33249

All core blocks define apiVersion with the number 2. We also used number in all examples in documentation and in the initial dev note https://make.wordpress.org/core/2020/11/18/block-api-version-2/.

#84 @ryelle
3 years ago

Yesterday, in r11097, I increased the error level of block.json issues, because most issues in that category will prevent block-plugins from appearing in the directory. The apiVersion was always a string, but it was considered a recommended change before. I agree this is incorrect and should be updated to a number.

We need to define the block type schema to validate against it - is there a core-defined schema that we could use to avoid these kinds of mistakes? (I also asked this in slack)

@ryelle
3 years ago

Update apiVersion to number, use rest_validate_value_from_schema to validate json object

#85 @ryelle
3 years ago

It looks like I'd need to write a validate_number function to add number support to the validator, so I've tried an different approach in 5303.diff - removing all our custom validation in favor of rest_validate_value_from_schema. I think this does everything we want (and some, since it would support all the property definitions that the core schema validates), the only drawback is that it returns on the first invalid value, rather than listing them all.

#86 @welcher
3 years ago

I like to submit that the schema no longer be in the plugin but rather pulled from https://json.schemastore.org/block.json or, once it's ready, the new package being proposed to manage the schemas for Gutenberg - https://github.com/WordPress/gutenberg/issues/35927.

I'd be happy to open a new ticket if that makes more sense.

#87 @ryelle
3 years ago

Thanks for sharing that issue, a @wordpress/schemas package would be a great help here. Once that's released, we should use it here.

@gziolo
3 years ago

Invalid errors reported by Block Plugin Checker

#88 @gziolo
3 years ago

https://schemas.wp.org/trunk/block.json is ready to use. I see that the block plugin checker shows three errors for the block scaffolded with the up-to-date local copy of npm @wordpress/create-block which follows the block schema. I included the screenshot and I can also include the zip file if you want to use it for testing purposes. I'm concerned only about the issues reported for the block.json file. The prompt for the unique namespace is a valid one and I appreciate including this check.

#89 @ryelle
3 years ago

In 11410:

Plugin Directory: Update block.json validation and discovery in block plugins.

Start using https://schemas.wp.org/trunk/block.json to validate the block.json. This fixes a few mismatched field types between our validator and the official expected format. This also updates the block discovery code to merge found blocks (if a block is registered in both PHP and JS, for example).

Props gziolo, welcher, jeffpaul.
See #5303, #5971.

#90 @ryelle
3 years ago

In [11410], I removed all the custom schema checking for the remote schema & rest_validate_value_from_schema, which almost works perfectly — the exception is a core bug #core54740. There's a workaround for it now, but hopefully the core issue can be fixed.

I did not cache the schema file locally, arguably it should be cached for a short time if this is called often. Any thoughts @tellyworth?

#91 @welcher
3 years ago

This is awesome! I just tested it with one of the plugins from the examples repo and the JSON validated as expected.

One thing I noticed was that there are some existing blocks using the gutenberg-examples namespace and even the full name of one of the example plugins gutenberg-examples/example-01-basic is being used. Perhaps we can add some logic to flag those as being used?

@welcher
3 years ago

@gziolo
3 years ago

Block Plugin Checker fixed

#92 @gziolo
3 years ago

I just wanted to confirm that all the issue are resolved now when scaffolding blocks with npx wp-create-block esnext-test --namespace gutenberg --no-wp-scripts in the Gutenberg repository. This is great news because it should continue to work correctly when we publish updates for WordPress packages to npm. Thank you for fixing it!

#93 @ryelle
3 years ago

In 11675:

Plugin Directory: Block Checker: Add gutenberg-examples to the disallowed namespaces list.

The gutenberg-examples/ namespace is used by the gutenberg-examples repo, submitted blocks should use their own namespace.

Props welcher.
See #5303.

#94 @ryelle
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

I've added gutenberg-examples to the "disallowed namespaces" list, so it will flag a warning like create-block/ does already.

With that, I'm going to close the ticket, as this has been up and running for a while now. If there are other issues with the block checker, they can be added as new tickets.

#95 @JeffPaul
3 years ago

Totally agree and really great, continued work on this @ryelle @dd32 @tellyworth (and anyone else I missed, if so sorry!), whether the rest of the internet feels like chiming in I'll at least say that I'm thankful for everything you've done on this here!

Note: See TracTickets for help on using tickets.