Update hook_help for Toolbar module

Trying to correct it according to standards

Thanks -- this looks good! A couple of things still need to be fixed:

a) Small standards fix: When referring to the module, say "the Toolbar module". "the" is missing in the last sentence of About.

b) I also noticed a small grammatical problem:

The drawer can be hidden/shown by clicking on its corresponding tab.

The previous sentences refer to several drawers, not one drawer. So this should probably say "Each drawer can be..."?

a) & b) suggestions are logical. I corrected them in this patch.

simpletest.me went ok & qa as well. Ready for comments and reviews.

Thanks! I think the help reads well now. However, the patch in #4 does not have the URL standards fixes from the patch in #2. Whoops! Just need to combine them. :)

The toolbar has changed quite a bit:

• It now consists of two rows of links, one being top level menu entries and the second being the top level adminisitration items as we know them.
• The second bar can be toggled to be displayed vertically, where each item can be collapsed into lower level admin item (emulating contrib modules like admin_menu a bit).
• On narrow screens the second bar is displayed vertically by default.

This is not currently mentioned in the help text.

We just had a change to hook_help, on this issue: #2183113: Update hook_help signature to use route_name instead of path.

Here is the change record: https://drupal.org/node/2250345

This patch will need a reroll for this change.

@jhodgdon - Based on the suggestion in #7 and the way new toolbar works, I have made some changes in the help text.

Please see if it looks good to you as well.

Thanks for the new patch!

I think we should keep the description in "About" fairly general, not getting into details about the two rows of links etc. The first sentence, in particular, should just give a short description of what the module does. The details should go in Uses sections, as they were before.

Also, we should not have sections explaining how to use the Shortcut module. We should *mention* the Shortcut module and link to its help page for details, and explain where/how the shortcuts can be displayed within Toolbar, but we shouldn't give details about how to edit shortcuts or add a shortcut link in the Toolbar help.

@jhodgdon - Thanks for your feedback.

I have updated the About / Uses sections based on your feedback. I have also corrected URL format and online help link.

Please review.

Thanks! I still think this needs some work... There is still a big section that is specific to Shortcuts, but it shouldn't be (actually the text there applies to either the main menu or the shortcuts, whatever is displayed in the "drawer" that is currently visible, doesn't it?).

So... I just tried this module out (hadn't played with it in a while). Here's the behavior I think we should be documenting in the help:

a) Toolbar displays "tab" and "tray" content from other modules. Core modules such as Shortcut, Edit, and Menu provide tabs and trays.
b) Tabs are usually buttons, and are displayed in a bar across the top of the screen. Some tabs are simple buttons that execute an action (such as starting Edit mode); other tabs toggle which tray is open.
c) Trays are usually lists of links, which can be hierarchical like a menu. If a tray has been toggled open, it is displayed either vertically or horizontally below the tab bar, depending on the browser width. In wide browser widths, the user has the ability to toggle from vertical to horizontal, using a link at the bottom or right of the tray; hierarchical menus only have open/close behavior in vertical mode.

I'm not suggesting this exact wording, but I think this is what needs to be documented for this module.

@jhodgdon - Suggested text looks good. I have placed the Tabs/Trays descriptions under Terminology section like Breakpoint module.

I have also removed Shortcuts section, as suggested. Please see the revised help for Toolbar module.

Better, thanks!

So... In the first line of About:

"The Toolbar module displays a bar containing top-level administrative components across the top of the screen."

We need a better summary...

Maybe:

The Toolbar module provides a toolbar for site administrators, which displays tabs and trays provided by the Toolbar module itself and other modules.

(since you've defined tabs and trays below anyway). I think the text in About in the Terminology section is very good.

So, then the Uses section... I wonder if we even need it, since the About section, with its terminology, describes the uses? Let's just get rid of it. I do not think it is adding any information.

Thanks!

Sounds better! I have made the required changes.

Please review.

Looks good to me! Thanks!

Checked this one out to give it additional community review, and had some grammar and text cleanup thoughts. I worked off of #15 patch.

I made edits to the tabs and trays definitions, and felt the following text might be a little cleaner:

Tabs are buttons, displayed in a bar across the top of the screen. Some tabs execute an action (such as starting Edit mode), while other tabs toggle which tray is open.

Trays are lists of links, displayed either vertically or horizontally below the tab bar, depending on the browser width. In wide browser widths, the user has the ability to toggle from vertical to horizontal, using a link at the bottom or right of the tray. Tray links only have open/close behavior in vertical mode.

Feel free to incorporate or not. I added a patch just in case. This was just based on how the content hit me the first time I read through it.
Cheers

Hi paboden! Welcome to this issue and contributing!

A couple of things:

a) If you add a new patch to an issue, you need to set the status back to "Needs review" because it is a new patch and needs reviewing, so the "reviewed and tested" status is not correct.

b) When adding a patch to an issue that already had a patch, an interdiff file is also requested: https://drupal.org/documentation/git/interdiff

So... Anyway, I reviewed your new patch.

I like the first change you made, to the part about tabs.

I am not as positive about the second change you made. I think the information about hierarchical lists is important, and it's also important to note that the hierarchical ones do not work as well in horizontal mode (which is counter to what people might expect since many horizontal menus in general on the web have drop-down behavior). And changing it to say "Tray links only have open/close behavior in vertical mode." is confusing. The previous text was, I think, better?

I am also not sure about removing the "usually" in the trays (in tabs I think it is OK). If you look at the developer documentation for the Toolbar module, you will see that modules can return whatever content they want for a tray. It doesn't have to be a list of links as far as I can tell. See:
https://api.drupal.org/api/drupal/core!modules!toolbar!toolbar.api.php/f...

Also... I am seeing now that in the hook_toolbar documentation, the "tabs" we have documented here are also called "toggles". Maybe that would be a better term to use in this help as well? It does seem clearer than "tabs"

Just to move this forward, I'm attaching a new patch which incorporates paboden's text for "Tabs" and amitgoyal's earlier text for "Trays," per jhodgdon's review.

Jennifer, I'm not sure I understand what you're saying in the last part, about tabs being called toggles. The hook documentation says:

The tray may be associated with a toggle in the administration bar. The toggle shows or hides the tray and is optimized for small and large screens. To create this association, hook_toolbar() returns one or more render elements of type 'toolbar_item', containing the toggle and tray elements in its 'tab' and 'tray' properties.

To me that says the element to show/hide trays is a "toggle" but the elements within a tray is a "tab." Directly under that text it says that the first property available is "'tab': A renderable array."

But maybe I'm not understanding what you're meaning?

I do think it would make more sense to call this something more generic -- "button" would make more sense to me than "toggle," especially since the first line of the help text says "Tabs are buttons." But in that case I think we should change the name of the array from "tab."

Anyway, here's a revised patch and interdiff.

I think this help is looking very good!

The only thing that I think is missing is that it doesn't explicitly mention that you can only have one tray open at a time, which I am pretty sure is true: if you open a try, any other open trays will close, right?

And maybe we should explicitly say that if you display a hierarchical tray horizontally, you only see the top level of links? It is kind of implied but maybe not quite obvious from the text that is there... There is no harm in making it obvious. :) So maybe take this phrase:
; hierarchical menus only have open/close behavior in vertical mode.

and make it into a separate sentence...

Hierarchical menus only have open/close behavior in vertical mode; if you display a tray containing a hierarchical menu horizontally, only the top-level links will be available.

Please review updated patch with #20 changes.

That looks good... it doesn't include this change though from #20:

... it doesn't explicitly mention that you can only have one tray open at a time, which I am pretty sure is true: if you open a try, any other open trays will close, right?

I had to re-roll the patch because the Shortcut URL had been changed from a relative path to using drupal::URL, but that will be deleted by this patch anyway...

I added

Only one tray may be open at a time. If you click another tab, that tray will replace the tray being displayed.

On my plain Drupal 8 installation, I could verify that only one tray can be open at a time. Clicking on each tab would open the corresponding try, replacing the open tray. In a base Drupal 8 installation, there are no tabs without trays, so I couldn't test what happens if you click on a tab that is an action.

Fixed patch to base off 8.0.x and not result of patch #21.

This patch looks great to me, thanks!

Committed 24bc995 and pushed to 8.0.x. Thanks!