Make enable/disable settings less ambiguous

Thank you very much Charles (@charles belov), as always, for your very constructive and helpful feedback.

No problem and thanks a lot for the suggestions. 👌

✅ OK for the requirements:

Expected result:
a. The label for the check box corresponds to what happens when you check the box.
b. Hover intent is two words

✅ OK for the labels on the Admin Toolbar Tools settings page at /admin/config/user-interface/admin-toolbar-tools:

Enable/Disable local tasks display
Local tasks such as node edit and delete.

🛑 KO But I can't seem to find:

Enable/Disable the hoverintent functionality
Check it if you want to enable the hoverintent feature.

I also searched module's code base and could not find these labels...
Could you please provide more details on where these textual elements could be found?

Otherwise, could you please review the Admin Toolbar settings form at /admin/config/user-interface/admin-toolbar?
In particular, the text hoverIntent appears multiple times.... See screenshot below:

Initially the idea behind using a single word was probably to reference the Javascript library and its corresponding effect.

However, if you have recommendations on replacing any occurrences of the word, with two words in the form, they would surely be greatly appreciated.
We would probably need to look more generally at replacing any occurrences of the word in form labels.

Just to let you know: We've already done a few rounds of improvements on these forms with @ressa and he could definitely be of great help to review and validate textual changes to these forms.
 

Ideally:
First, we would like to collect as many required changes as possible, so they could all be processed at once and tested exhaustively in a single round: Trying to minimize the amount of back and forths we might need on an issue like that.

Please let us know your suggestions of adjustments to be made to the forms and we will be glad to make the corresponding code changes.
Thanks in advance for your feedback and help!

Thanks @charles belov, these are great suggestions, and they improve the text.

But like @dydave suggests, ideally we should give the whole page a look-over, and tighten up the text strings more methodically.

I have added the strings in the Issue Summary, from the latest 3.x dev-release, maybe you can add your suggestions there? Thanks!

Thanks, I just meant that to make it more systematic, it might be best if you could share your opinion on the individual strings, as I listed them in the Issue Summary here:

Remaining tasks

Go through the settings strings on the page, and check if they can be improved. Either add "OK", or the new suggestion below:"

• The Admin Toolbar module provides [...]

But maybe your comment could work as well, let's see what @dydave thinks.

Actually, ideally you could create an MR, so that we can review your suggestions directly in the user interface ... that way, we don't have to guess which strings you want to update.

Thanks @charles belov, and for updating the documentation as well.

Remove list of strings from the Issue summary.

Super nice feedback and help on this issue Charles (@charles belov) and @ressa, once again! 🤩
Great job! 🥳
Thanks a lot! 🙏

✅ OK
I have reviewed all the changes suggested at #5 and the ones in the Issue Summary and rolled them in merge request !198 👍

See additional commit:
https://git.drupalcode.org/issue/admin_toolbar-3564708/-/commit/fb24122d...

I "think", all the changes mentioned in this issue should now be included in this merge request, but it would be great if you could both maybe take another look... thus, moving issue to Needs review 🤞

Charles (@charles belov), could you please take another look at the forms and double check if we could have missed anything?

@ressa, if you could maybe try grepping and such, to see if we could have missed anything else, it would be super helpful as well. 😊
I don't think it would be necessary to change 'hoverIntent' in code comments. 👌

Feel free to add changes and edit directly any of the files from the merge request.

Any comments, reviews or testing feedback would be greatly appreciated. 🙏
Thanks in advance!

Thanks @dydave and @charles belov, these are great suggestions. I added two comments in the MR.

This is what I suggest about the shortcuts in bold:

Also, I see that both "hover intent" as well as "hover-intent" is used in the MR. Is this intentional @charles belov?

Adding image.

Thanks for the fast feedback @charles belov. Great suggestion with using "show/hide-toolbar shortcut", and I have updated my suggestion.

If you agree with my suggestions in the MR, you can click "Apply suggestion" which will update the MR, followed by "Resolve thread", to signify that the changes have been made, and there are no remaining tasks ("unresolved").

Thanks for clarifying the "hover-intent" and "hover intent" differences. About seeing the different instances, they can be seen for example in the raw diff: https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/198.diff

Great work on this Charles (@charles belov) and @ressa! Thanks a lot! 🙏

I'll take a step back and let Charles review the merge request, as suggested by @ressa, if possible, to apply suggestions and resolve threads.

I'm personally very happy with the suggested changes, putting the focus on non tech savvy users.
The forms definitely look much more refined compared to where they were a year ago 🥳

Once you're both happy with the changes in the MR, I'll be happy to hit the merge button. 😊

Thanks again!

Thanks for fast feedback @dydave and @charles belov :)

I do remember how strange I thought the UI for "Suggest change" was, and I should have linked to the Suggest changes GitLab doc page, my bad. There will be a lot of trial and error when learning to use the GitLab UI, and we have all been there.

I have added a new section to the "Editing a GitLab merge request" doc page called Accepting a suggested change. Since it's still fresh in your mind, feel free to clarify or expand the content, if you think it could be helpful for a user new to the concept.

I agree with @dydave that it's a clear improvement, and the MR is ready.

Thanks a lot Charles (@charles belov) and @ressa, once again! 🙏

After doing another quick review of the merge request and a round of testing locally, since everything seemed to work as expected and all the jobs and tests were still passing 🟢, I went ahead and merged the changes above at #22. 🥳

This ticket is a great example of a constructive collaborative effort resulting in concrete changes bringing value to the community. 🤩
Additionally, it's been a great learning experience in terms of vocabulary guidelines and definitions. For example, I have a much better idea of how an 'On/Off' (boolean) checkbox config field label should be formatted and phrased.

At this point, we could probably consider that all the work expected in this ticket should have been completed, thus, marking as ✅Fixed, for now.
Feel free to let us know if you spot anything else or have more suggestions of improvements, it would definitely be very helpful.

Let's keep moving the work in other issues as well. 😅
Thanks again very much for your precious help making the module constantly better! 🙏

Special thanks to @ressa, once again, for the great documentation and use of Gitlab Suggestions.
I'll make sure to point contributors to the links and docs in this issue. 👍

Thanks @dydave for a fast action, as always!