Fix module description, hook_help(), and document module scope in *.api.php file

#2782891: The Page Title block's title behaves in a confusing way with Settings Tray and the Help block incorrectly has Settings Tray styling is in now, so unpostponing.

1. +++ b/core/modules/outside_in/outside_in.api.php
   @@ -10,6 +10,19 @@
   + * The goal of the Settings Tray module is to make administering blocks easier,
   + * and therefore to make the site building experience faster and more pleasant.
   

   This doesn't doesn't mention anything about expose configuration that is related to blocks but is not the block configuration itself, for instance menu edit form or site title.

2. +++ b/core/modules/outside_in/outside_in.info.yml
   @@ -1,6 +1,6 @@
   -description: 'Provides the ability to change the most common configuration from the Drupal front-end.'
   +description: 'In-place editing of block configuration.'
   

   Again doesn't mention related configuration.

Changing to new settings_tray.module component. @drpal thanks for script help! :)

Needs reroll at least for changes in #2803375: Rename Outside-in module to "Settings Tray" for real

Re-rolled.

Removed out-of-context change.

@Jo Fitzgerald thanks for the re-roll!

+++ b/core/modules/settings_tray/settings_tray.info.yml
@@ -1,6 +1,6 @@
-description: 'Provides the ability to change the most common configuration from the Drupal front-end.'
+description: 'In-place editing of block configuration.'

Should we add something about surfacing related configuration here or in the help text? Because it does more that just editing block configuration.

@Wim Leers ok sounds good!

Great to see these docs being added! I think they have all the right information.

We can remove some words from this to make it more digestible. xjm uncaps her red copy-editing pen.

1. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * The goal of the Settings Tray module is to make administering blocks easier,
   + * and therefore to make the site building experience faster and more pleasant.
   

   Replace with:

   The Settings Tray module makes administering blocks easier.

   And it can be joined with the next paragraph.

2. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * It achieves this by building on the infrastructure that the Contextual
   + * Links module provides: Settings Tray provides a "Quick Edit" contextual link
   

   Replace with:

   It provides a "Quick Edit" link for blocks using the Contextual Links
   module.

3. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * for blocks. Clicking this contextual link opens the Settings Tray, which
   

   Replace with:

   Clicking this link opens the off-canvas Settings Tray.

4. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * allows to change the rendered contents of the block by showing all relevant
   

   Replace "allows to change" with "allows changing".

5. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * - for every block, one can configure whether to display the block title or
   + *   not, and optionally override it (block configuration)
   + * - for menu blocks, one can configure which menu levels to display (block
   + *   configuration) but also the menu itself (menu configuration)
   + * - for the site branding block, one can change which branding elements to
   + *   display (block configuration), but also the site name and slogan (simple
   + *   configuration)
   

   Each bullet should be capitalized and end with a period.

6. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -10,6 +10,29 @@
   + * The Settings Tray uses a variant of a standard dialog: an off-canvas dialog.
   

   This paragraph doesn't really belong in this section. It could be added to the next section instead.

7. +++ b/core/modules/settings_tray/settings_tray.info.yml
   @@ -1,6 +1,6 @@
   -description: 'Provides the ability to change the most common configuration from the Drupal front-end.'
   +description: 'In-place editing of block configuration.'
   

   Replace with:

   Provides a sidebar to configure blocks on the page.

   (The module descriptions are supposed to start with a verb. In the past couple years there was a big initiative to make them all follow this pattern.)

8. +++ b/core/modules/settings_tray/settings_tray.module
   @@ -18,10 +19,14 @@ function settings_tray_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('To edit blocks in place, you can either enable \'edit mode\' in the toolbar and click the block, or choosing Quick edit from the contextual links of a block (see the <a href=":contextual">Contextual Links module help</a> for more information about how to use contextual links).', [':contextual' => \Drupal::url('help.page', ['name' => 'contextual'])]) . '</p>';
   

   "Enable edit mode" is a bit confusing, "Edit" should be capitalized, and the sentence is a bit run-on. I'd say:

   To edit blocks in place,
   either click the Edit button in the toolbar and then click on the block,
   or choose Quick edit from the block's contextual link. (See the Contextual Links module help for more information about how to use contextual links.

9. +++ b/core/modules/settings_tray/settings_tray.module
   @@ -18,10 +19,14 @@ function settings_tray_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('This will open the Settings Tray for this block, where a compact form will be displayed, with configuration that controls what the block shows. After saving, the page reloads, and the effects are visible.') . '</p>';
   

   I wouldn't start a new paragraph here; if we do, it's unclear what "this" refers to. I'd instead say:

   The Settings Tray for the block will open in a sidebar, with a compact form for configuring what the block shows. Save the form and the changes will be immediately visible on the page.

@xjm thanks for the review
1. fixed
2. fixed
3. fixed
4. fixed
5. fixed
6. The next section is about the API provided for this module. The off-canvas dialog is not part of the API. I am wondering if we need this at all.
7. fixed
8. fixed
9. fixed

Test failed b/c of "CI Job missing" 🤔 Retesting

6. The next section is about the API provided for this module. The off-canvas dialog is not part of the API. I am wondering if we need this at all.

Yeah maybe just remove it, so long as the API section has an @see to offcanvas?

ok done

Looks great!

Build timed out (after 110 minutes). Marking the build as aborted.

retesting

1. @Wim Leers I think you are right in #25.

   I removed off-canvas from here:

   link opens the off-canvas Settings Tray which allows changing the

   Then the only instance of "off-canvas" in the this file is followed by a @see to the off-canvas.es6.js file.

   I think using "sidebar" is ok in settings_tray.info.yml because this will be seen in the modules page by site builders and we can't assume they will look at the code at so I think "sidebar" is probably the most common and recognisable word for this.

2. In updating the settings_tray.api.php I found a couple other uses of off-canvas that were out of date.

   We were referring to setting the "off-canvas form" in the annotation but since #2904134: Settings Tray uses the off-canvas dialog type, but "off_canvas" is not an accurate form plugin name, "settings_tray" is settings_tray is used in the annotation.

1. I think the link is "Quick edit" (which is the correct casing) but in one place the docs have "Quick Edit". Let's correct the latter wherever it appears. ("Quick Edit" with title case should only be used when referring to "the Quick Edit module").

2. +++ b/core/modules/settings_tray/settings_tray.api.php
   @@ -33,16 +54,16 @@
   + * Finally, blocks that do not specify an settings_tray form using the
   

   "an settings_tray form"

3. +++ b/core/modules/settings_tray/settings_tray.module
   @@ -18,10 +19,15 @@ function settings_tray_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('To edit blocks in place, either click the <strong>Edit</strong> button in the toolbar and then click on the block, or choose "Quick edit" from the block\'s contextual link. (see the <a href=":contextual">Contextual Links module help</a> for more information about how to use contextual links).', [':contextual' => \Drupal::url('help.page', ['name' => 'contextual'])]) . '</p>';
   

   The sentence beginning with "see" needs to be capitalized. Edit: Also, very minor nitpick, but the final period can go inside the paretheses since the entire sentence is part of the parenthetical.

Accidentally deleted one bullet of my review so I'll have another comment shortly.

+++ b/core/modules/settings_tray/settings_tray.api.php
@@ -10,9 +10,30 @@
+ * The Settings Tray module makes administering blocks easier. It provides a
+ * "Quick Edit" link for blocks using the Contextual Links module. Clicking this
+ * link opens the Settings Tray which allows changing the rendered contents of
+ * the block by showing all relevant configuration (minus block visibility
+ * conditions). This means blocks can be modified without leaving the page.

So, yeah, what I noticed is that this only mentions the "Quick edit" contextual link and not the "Edit" mode button and interactive block hovers. We could mention both; however, since this is API documentation and not user documentation, I don't think we actually need to explain the user interface. We just need to explain what the developer can do with it.

So I think we can just say:

The Settings Tray module allows blocks to configured in a sidebar form without leaving the page. For example:
- [Same existing list here]
-
-

Block visibility conditions are not included the sidebar form.

+++ b/core/modules/settings_tray/settings_tray.api.php
@@ -10,9 +10,30 @@
- * By default, every block will show its built-in form in the Settings Tray.
+ * By default, every block will show its built-in form in the Settings Tray
+ * which uses the off-canvas dialog.

Oh this paragraph is also weird. I'd say:
"By default, the Settings Tray shows any block's built-in form in the off-canvas dialog."

@xjm thanks for the review

#32

1. The link is "Quick edit". Fixed. There 2 case of "Quick Edit" in the .module file but not in the areas we are changing. Let me know if you want me to fix those in this patch also
2. Fixed. I also fixed a line a couple down where the word "this" could be move to the above line.
3. Fixed

#33 So tragic

#34
Fixed

#35
Fixed

The link is "Quick edit". Fixed. There 2 case of "Quick Edit" in the .module file but not in the areas we are changing. Let me know if you want me to fix those in this patch also

Nah it looks like those two are in inline comments, so out of scope here. Thanks!

Looks good; I'll leave for someone else to review though since I proposed the edits.

Ah one thing is missing from the patch: the improvement for the module description. (It is in the issue title.)

We can simplify the description a bit. My proposal is:
Allows changing the most common configuration from the Drupal front-end.

I can't read. What's already in the patch is better.

OK RTBC!

A couple of nitpicks that stood out:

The Settings Tray module allows blocks to configured in a sidebar form

...to "be" configured

 providing an
- * off-canvas form:
+ * settings_tray form:

providing "a" settings_tray form.

Then, more of a philosophical question:

-description: 'Provides the ability to change the most common configuration from the Drupal front-end.'
+description: 'Provides a sidebar to configure blocks on the page.'

Here (and elsewhere), we're removing language about Settings Tray being a generic way to configure common properties, and making it about blocks specifically. While I realize that's what's currently been implemented in the "1.0 release", the end goal all along was for this to be a generic UI component that could be hooked into by a variety of core + contrib modules. (See https://dri.es/examples-of-how-to-make-drupal-outside-in)

Is there a particular reason we're stripping this back? At a glance, that seems to make it less appealing to contrib + other parts of core to integrate with it, which is kinda the opposite of what we're going for here? (And also unclear why it's called "Settings Tray" and not "Quick Block Config" or something—NOT that we should rename it again! :D)

only nitpicks

Here (and elsewhere), we're removing language about Settings Tray being a generic way to configure common properties, and making it about blocks specifically. While I realize that's what's currently been implemented in the "1.0 release", the end goal all along was for this to be a generic UI component that could be hooked into by a variety of core + contrib modules. (See https://dri.es/examples-of-how-to-make-drupal-outside-in).

So I mentioned this to Angie briefly already but: from my perspective it's actually the offcanvas library that is the generic UI component (and we already have a great example of other "Drupal becoming outside in" with the Layout Builder, which uses the offcanvas library but does not depend on Settings Tray). Everything in Settings Tray really is about blocks: the UI interaction, the under-the-hood implementation, the API provided to contrib. It can be extended by core and contrib... if you (as a developer) want to hook into configuring blocks in an awesome user-friendly UI. If you (as a developer) don't want to hook into configuring blocks, you probably just want the offcanvas library. :)

My concern with the current description is that "Provides the ability to change the most common configuration from the Drupal front-end" is really not true at all (and also kind of vague and confusing). I'd say "the most common configuration" is content types, fields, views, etc... none of which can be configured with Settings Tray. Some day those things might be configurable in a different offcanvas implementation. If some day Settings Tray encapsulates several offcanvas implementations for different things, then we can update the module description, extend the API docs, etc. which would be the docs gate for that feature. :)

Ok, thanks for the clarification.

Conferred with @yoroy and he agreed with erring on the side of accuracy here.

Committed and pushed to 8.5.x. Thanks!