On #2148255: [meta] Make better D8 api.d.o landing page, linked to high-level overview topics, and put it in Core api.php files, we made a patch that included a stub Topic page for api.drupal.org (i.e., a @defgroup) titled:

Views overview

This can be found in file core/modules/system/core.api.php where it says

@defgroup views_overview

The documentation to go on this page needs to be written. The idea is:

a) Write a few paragraphs about the topic.

b) Link to more detailed documentation on
https://drupal.org/developing/api/8

c) If the more detailed documentation does not yet exist, create stub page(s), link to the stub pages, and add a note to this issue stating that the stub pages need to be filled out.

d) If the topic has related classes, interfaces, and functions, add

@ingroup views_overview

to their documentation headers. That will make these classes etc. show up on the Topic page on api.drupal.org.

For more info -- documentation standards for @defgroup/@ingroup:
https://drupal.org/coding-standards/docs#defgroup

CommentFileSizeAuthor
#11 2216539-views-docs-fixup.patch4.11 KBjhodgdon
#4 interdiff.txt3.77 KBjhodgdon
#4 2216539-views-docs-overhaul-4.patch56.3 KBjhodgdon
#2 2216539-views-docs-overhaul-2.patch55.69 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Assigned: Unassigned » jhodgdon

Since this was filed, quite a bit of Views API docs have appeared on api.drupal.org.... will figure out how to integrate it.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Active » Needs review
FileSize
2216539-views-docs-overhaul-2.patch55.69 KB

Well. This ended up being a "down the rabbit hole" issue.

There were a lot of topics for Views already... The objective here is that developers would be able to have links to information they'd need in order to interact with the Views module API. So... Here is what I did in this patch:

1. We had put in a skeleton Views Overview topic, linked from the main api.drupal.org D8 landing page (that was the subject of this issue). I moved this into the views.api.php file, since that seemed a better home than core.api.php in the system module. I also filled it out with links to other topics and hooks.

2. I looked through the list of existing Views topics on api.drupal.org. There was on on Ajax that seemed like it was only documenting some internals of how Views does Ajax. So I removed this topic -- the purpose of topics is for developers to discover APIs, not to document internals of modules, so it seemed like we did not need this one.

3. The views.api.php file had a plugin overview topic, which attempted to list all the types of plugins. But it was incomplete... so what I did was remove all that documentation there and put it onto the individual plugin type topics instead. Then each of those plugin type topics was given @ingroup views_plugins so that it would (a) show up on the Views Plugins overview page (it will list topics that are @ingroup, just like it will list functions and classes), and (b) each individual type topic would have a "related topics" link back to the overview page.

4. A few plugin types did not have topics. I added topics for them.

5. Developers of plugins need to know: the annotation class, the plugin base class and/or interface, and the plugin namespace. So I put this information into each plugin type topic.

6. Developers go to the list of all Annotations to discover types of plugins. So annotations need to link to the documentation listed in #5. So, I added @ingroup (specific plugin topic) to each of the annotations. This covers
#2288517: Views annotation classes need @ingroup added, which has now been marked as a duplicate of this issue.

7. I also added a @see to the Plugin API topic to each of the specific plugin type topics.

8. I did a little bit of additional standards cleanup and clarity/typo cleanup in some docs headers (hard to resist). Mostly just the @file headers (Definition of vs. Contains), and making all the topics have similar layout/style/wording. As one note on that... The Field and Filter plugins had some documentation on what you can put into hook_views_data(). In one it was on the base class, and in the other it was on the topic. I moved them both to the topic. Also, one of the plugin base classes had a bunch of information on how to do annotations; I took that out as we have centralized docs on this now (or will as soon as a patch is committed).

Whew! I think that is all my notes.... And I think after this patch, plus or minus any typos or bits of misinformation that hopefully the reviewer(s) will find, we'll have considerably better Views documentation for developers. There is more that could be done that I noticed ... but at least developers should be able to get what they need. I hope.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/modules/system/core.api.php
@@ -401,23 +401,6 @@
 /**
- * @defgroup views_overview Views overview
- * @{
- * Overview of the Views module API
- *
- * @todo write this
- *
- * Additional documentation paragraphs need to be written, and functions,
- * classes, and interfaces need to be added to this topic. Should link to all
- * or most of the existing Views topics, and maybe this should be moved into
- * a different file? This topic should be an overview so that developers know
- * which of the many Views classes and topics are important if they want to
- * accomplish tasks that they may have.
- * @}
- */

    i wonder who moved that in, given that this probably did not existed in views previously 

  2. +++ b/core/modules/views/src/Annotation/ViewsArea.php
@@ -10,9 +10,11 @@
+ *
+ * @ingroup views_area_handlers
+ *

    i am a bit confused about these @ingroup annotations ... i thought they existed to list all of those handler types 

  3. +++ b/core/modules/views/src/Annotation/ViewsWizard.php
@@ -12,9 +12,12 @@
  * @see \Drupal\views\Plugin\views\wizard\WizardPluginBase
+ * @see \Drupal\views\Plugin\views\wizard\WizardInterface

    Is there a standard to remove references to base classes? 

  4. +++ b/core/modules/views/src/Plugin/views/access/AccessPluginBase.php
@@ -14,10 +14,17 @@
+ * Access plugins are responsible for controlling access to the view.
+ *
+ * Access plugins extend \Drupal\views\Plugin\views\access\AccessPluginBase,
+ * implementing the access() and alterRouteDefinition() methods. They must be
+ * annotated with \Drupal\views\Annotation\ViewsAccess annotation, and they
+ * must be in namespace directory Plugin\views\access.
+ *
+ * @ingroup views_plugins
+ * @see plugin_api

    <3 

  5. +++ b/core/modules/views/src/Plugin/views/argument_validator/ArgumentValidatorPluginBase.php
@@ -14,7 +14,23 @@
+ * Plugins for validating views contextual filters.
+ *
+ * Views argument validator plugins validate contextual filters (arguments) on
+ * views. They can ensure arguments are valid, and even do transformations on
+ * the arguments. They can also provide replacement patterns for the view title.
+ * For example, the 'content' validator verifies verifies that the argument
+ * value corresponds to a node, loads that node, and provides the node title
+ * as a replacement pattern for the view title.
+ *
+ * Argument validator plugins extend
+ * \Drupal\views\Plugin\views\argument_validator\ArgumentValidatorPluginBase.
+ * They must be annotated with
+ * \Drupal\views\Plugin\Annotation\ViewsArgumentValidator annotation, and they
+ * must be in namespace directory Plugin\views\argument_validator.
+ *
+ * @ingroup views_plugins
+ * @see plugin_api
  */

    Really nice 

  6. +++ b/core/modules/views/src/Plugin/views/display_extender/DefaultDisplayExtender.php
@@ -2,13 +2,15 @@
+ * Default display extender plugin; does nothing special.

    What about "does nothing."? This makes it a little clearer for me. 

  7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php
@@ -16,13 +16,20 @@
+ * Exposed forms are used for filters and sorts that are exposed to site
+ * visitors. Exposed form plugins handle the rendering, validation, and

    Should we mention pagers as well? 

  8. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
@@ -17,24 +17,33 @@
+ * - click sortable: If TRUE, this field may be click sorted.

    Note: click sortable is the default now 

  9. +++ b/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php
@@ -14,30 +14,33 @@
+ * - $no_operator: Disable the possibility of using operators.

    Did you found an issue to rename that variable?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
FileSize
2216539-views-docs-overhaul-4.patch56.3 KB
interdiff.txt3.77 KB

Thanks for the review in comment #3!

Regarding point 1 -- we created the views overview in another issue, when we created the new and improved api.drupal.org landing page for Drupal 8. Lots of "stub" topics were added to core.api.php, including this one.

Regarding point 2 -- The @ingroup did not exist in the Annotation classes. Annotation classes are an important way for people to discover plugin types, so they need to be linked to relevant other classes and documentation. Since Views provides this documentation in the "Plugin type foo" topics, the Annotation classes need to have these @ingroup lines to link to those topics. See #2269389: [meta] Make sure plugin developer info is discoverable for more information on this subject.

Regarding point 3 and point 4 - no, in fact just the opposite. See above issue. Interfaces, base classes, plugin manager classes, and annotations (if they exist) all need to be linked together so that plugin developers can find them. Remember, all of this topic/@see/@ingroup documentation is for developers who need to be able to find information on how they can create plugins.

Regarding points 6, 7, 8: Addressed.

Regarding point 9, it looks like $no_operator and $always_required variables should both be renamed. Are you saying there is already an issue? Hopefully it will also fix the docs when/if it happens.

Other changes in this patch:
- I rerolled the patch -- views.api.php segment didn't apply -- this reroll happened before the interdiff file.
- I found an error in the Exposed plugins docs, fixed that.
- I just noticed the "views_objects" topic, which contains only one class. This seems pointless. There are no references to this @defgroup anywhere, and we already have a LOT of topics on api.drupal.org. So I removed it, and instead added a link in the Views Overview topic to this one class.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Thank you. I don't really have time to review that in more detail but for sure, this is already a huge step forward.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Wow, overhaul is right. :) These changes all look good to me. A little sad to be losing that demo_pager code, but I can understand not wanting to do that as a one-off.

Committed and pushed to 8.x. Great work!

  • webchick committed 0843922 on 8.x 
    Issue #2216539 by jhodgdon, dawehner: Fill in @defgroup/topic docs for...
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Thanks for doing this, sorry I didn't review sooner. Daniel did admit he didn't really look though.... I spent a bit of time looking through:

  1. +++ b/core/modules/views/src/Plugin/views/access/AccessPluginBase.php
@@ -14,10 +14,17 @@
+ * annotated with \Drupal\views\Annotation\ViewsAccess annotation, and they

    The actual class is not relevant nowadays, as you don't need to use annotation classes anymore. Maybe just "They must be annotated with @ViewsAccess" ? 

  2. +++ b/core/modules/views/src/Plugin/views/access/AccessPluginBase.php
@@ -14,10 +14,17 @@
+ * must be in namespace directory Plugin\views\access.

    This looks strange, it's not a namespace directory. I know what you mean though. I think just namespace is good here. 

  3. +++ b/core/modules/views/src/Plugin/views/area/AreaPluginBase.php
@@ -12,16 +12,20 @@
+ * Area handler plugins extend \Drupal\views\Plugin\views\area\AreaHandlerBase.

    Area handlers have to implement a render() method at the very least. 

  4. +++ b/core/modules/views/src/Plugin/views/area/AreaPluginBase.php
@@ -12,16 +12,20 @@
- * @ingroup views_area_handlers

    Should we be removing the group for this base class? 

  5. +++ b/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php
@@ -17,12 +17,23 @@
+ * @ingroup views_plugins

    Here has a group. 

  6. +++ b/core/modules/views/src/Plugin/views/argument_default/ArgumentDefaultPluginBase.php
@@ -14,7 +14,20 @@
+ * Argument default plugins provide default values for contextual filters.

    Implementations for these should really be providing a getArgument() method. 

  7. +++ b/core/modules/views/src/Plugin/views/argument_default/ArgumentDefaultPluginBase.php
@@ -14,7 +14,20 @@
+ * This is useful for blocks and other display types lacking a natural argument

    "This can be useful..", otherwise I think it makes it sound like this is the main use case. 

  8. +++ b/core/modules/views/src/Plugin/views/argument_validator/ArgumentValidatorPluginBase.php
@@ -14,7 +14,23 @@
+ * Views argument validator plugins validate contextual filters (arguments) on

    can we stick to arguments and then (contextual filters) as that is how it evolved in other places in the patch.

    Also, these should implement a validateArgument() method. 

  9. +++ b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php
@@ -14,10 +14,17 @@
+ * Cache plugins control the storage and loading of caches in Views, for

    "loading of caches in views" - huh? Loading of cached data maybe?

    Also, they don't really control the storage. This is delegated to the cache system. You could do this yourself if you override everything in the base class. Not too likely though. 

  10. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
@@ -17,24 +17,33 @@
+ *   array('identifier' => array('table' => tablename, 'field' => fieldname))

    This can be in the form identifier => fieldname too - which is the most common case. 

  11. +++ b/core/modules/views/src/Plugin/views/style/StylePluginBase.php
@@ -15,17 +15,28 @@
+ * Style plugins control a view is displayed. For the most part, they are

    control how

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Fixed » Needs work
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Thanks for looking at this more carefully in #8!

Regarding these comments:

(1) and (2) are how we are doing this for other plugins outside of Views, so I would prefer to leave it. The annotation reference needs to give the full namespace of the annotation class so people can find it, and the namespace given is *not* the full namespace, but a subdirectory under the implementing module's namespace.

(3)/(6) We can mention particular methods that have to be done, but ... aren't they already abstract on the base class? That would be the best way to clue people in to "you must implement this", not to add to this topic docs.

(4) It is inside a @defgroup @{ @} so the extra @ingroup is not needed.

(5) isn't inside a @defgroup @{ @} so it needs the @ingroup.

#7 I can fix.

#8... Really this is very confusing. I realize that prior to Views 7.x, they were called "arguments", but they have been "contextual filters" in the UI for some time now. I think you need to really consider getting rid of calling them "arguments" anywhere in the code, because you are just going to confuse people. Reversing the references in this doc will not help. Let's open a different issue to address this.

#9, #10, #11 I can fix.

I'll make a new patch shortly...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Needs review
FileSize
2216539-views-docs-fixup.patch4.11 KB

Decided you were right on #8 too. Here's a patch.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Fixed

Looks like all feedback's been addressed.

Committed and pushed to 8.x. Thanks!

  • webchick committed b343c57 on 8.0.x 
    Issue #2216539 by jhodgdon: Fill in @defgroup/topic docs for Views...

Status: Fixed » Closed (fixed)

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