It's my understanding the the canonical documentation for what I should be able to write in an annotation should be documented in the @Annotation class for that specific annotation type. So for example, core/modules/block/lib/Drupal/block/Annotation/Block.php is code that never actually gets executed but serves soley to document what I should be able to put into a Block Plugin annotation.

If this assumption is correct, then the file mentioned is missing some properities. Right now it includes 'id', and 'admin_labal', but a quick grep of core shows that there are also at least 'category' and 'derivative' keys that I could add to a block plugin annotation.

We need to add these to this class so they are documented, and so that others will know they exist and how/when/why to use them.

Here's an example of an annotation using undocumented properties:

 * @Block(
 *  id = "custom_block",
 *  admin_label = @Translation("Custom block"),
 *  category = @Translation("Custom"),
 *  derivative = "Drupal\custom_block\Plugin\Derivative\CustomBlock"
 * )

The 'category' key is used to determine which category to list the block under in the admin interface. What is 'derivative' used for? If someone can let me know I'm happy to roll a patch for this.

CommentFileSizeAuthor
#9 2248951-9-block_annotation_docs.patch1.36 KBeojthebrave
#6 2248951-6-block_annotation_docs.patch1.36 KBeojthebrave
#2 2248951-2-block_plugin_annotation_docs.patch709 byteseojthebrave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

I'm asking in IRC about the assumption. That is my understanding too, that the @Annotation classes need to document the properties a developer should use for annotation... if not, we need some additional docs added to the header for the additional properties, or an @see leading to the class or topic or whatever that documents them.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
FileSize
2248951-2-block_plugin_annotation_docs.patch709 bytes

Here's a simple patch that adds $category and $derivative documentation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Component: documentation » block.module
Status: Active » Needs review

Let's see if the Block maintainers will review this.

Status: Needs review » Needs work

The last submitted patch, 2: 2248951-2-block_plugin_annotation_docs.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Apparently, adding this to the annotation breaks something, as that test failure is in Block tests.

If we cannot add it this way, we could do something like this in the docblock for the annotation class:

 * Most blocks will also have these additional annotation elements:
 * - category: ...
eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
Status: Needs work » Needs review
FileSize
2248951-6-block_annotation_docs.patch1.36 KB

It looks like the Annotation\Block class is actually used, and whatever the properties are set as in the stub class become the default values for those annotations. As such, setting $categroy = '' causes the !isset() in BlockManager::processDefinition() which is used to ensure that every block has a category needs to get changed to an empty() check since the value of category for a block that doesn't specify it in it's annotation is now $category = "".

Lets see if this one passes tests.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

OK, we really need the Block maintainers to review this. I've asked tim.plunkett to review this. benjy is the other maintainer; he was not on IRC at the moment...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented 
+++ b/core/modules/block/lib/Drupal/block/Annotation/Block.php
@@ -32,4 +32,19 @@ class Block extends Plugin {
+  public $derivative = '';
 }

Can you add a blank line back to the end of the class? Otherwise this is fine, the !isset/empty change makes sense.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave commented
FileSize
2248951-9-block_annotation_docs.patch1.36 KB

Now with more blank lines.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 33223ad and pushed to 8.x. Thanks!

  • Commit 33223ad on 8.x by alexpott: 
    Issue #2248951 by eojthebrave: Missing documentation for block plugin...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.