Improvement help text Recipient field

I am bumping this to 8.0.x because of the backport policy. The same text appears in Drupal 8.

Hi dbazuin,

thanks for the patch! Almost there, but I think that the two things below needs to be tackled too.

1. +++ b/core/modules/action/src/Plugin/Action/EmailAction.php
   @@ -197,7 +197,10 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
          // We want the literal %author placeholder to be emphasized in the error message.
   

   This comment should be updated too.

2. +++ b/core/modules/action/src/Plugin/Action/EmailAction.php
   @@ -197,7 +197,10 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   +        )));
   

   Indenting seems wrong?

Looks good to me. +1

I will have a look at this.

Still have to apply the patch, sorry.

Steps to reproduce:

1. Enable actions module
2. Create advanced action: send email
3. Git apply -v Improvement-help-text-Recipient-field-2603236-11-D8.patch
4. Checked the text below "Recipient" input field

The email address to which the message should be sent OR enter [node:author:mail], [comment:author:mail], [user:mail], etc. if you would like to send an email to the author of the original post or to one or more users.

5. Text is correct. See screenshot:

Steps have been reproduced and the patch is working! See screenshot

This is indeed more correct but 3 lines of description makes this almost documentation :-)

What if we make the label more explicit, then we could make the description quite a bit shorter:

Label: Recipient email adress

Description: You can also enter [node:author:mail], [comment:author:mail], [user:mail], etc. to send email to the post author or to one or more users.

Back to needs work. I agree with yoroy that the proposed description is quite long.

@tomgilissen, Pls use the issue summary template (https://www.drupal.org/issue-summaries) to update the issue summary. It contains a number of standard headings which help you cover the various things we like to see in an issue summary. A screenshot with the current situation, to illustrate the problem, will also help.

Thanks,
the patch works, but the initially proposed sentence is rather convoluted, it's unclear how "or to one or more users." can be added. I think we generally avoid emphasizing two different option with a capital OR because that's not really accessible, and usually a sign that the sentence is a bit too big.

How about:

Enter either an email addresses or a token to send the email to the author of the original post. Available tokens are [node:author:mail], [comment:author:mail], [user:mail]. Separate several recipients with a comma.

I agree with ifrik. Her text suggestion sounds really good to me.

I like the suggestion by yoroy to change the label to: Recipient email adress

And also agree with ifrik to change the description:
Enter either an email address or a token to send the email to the author of the original post. Available tokens are [node:author:mail], [comment:author:mail], [user:mail]. Separate several recipients with a comma.

I think we should keep address singular; token is also singular.

Patch created for YesCT with tomgilissen. Cool.

Thanks for picking this up so fast again.

Could you still add the "Separate several recipients with a comma." as last sentence because it's not obvious to users that they can add several recipients otherwise.

Patch created for YesCT with tomgilissen. Cool.

Separate several recipients with a comma added.

Thanks,
I think we've got it now!

Just as a general remark for naming and numbering patches: Please keep the patch name as it is (even if it looks weird) because otherwise it looks like the work started from scratch again. And always add the comment number to the name and just change that accordingly for a new patch. This way, it's easy to find where in the issue a specific patch was posted.

Added the "after" screenshot

Retested this issue with patch Improvement_label_help_text_email_recipient_field-2603236-D8-2.patch
The result is like mentioned by alimac in #42

Well done Pepino710 ! : )

yoroy made an enigmatic comment on stage during the Live Commit so marking back to Needs Review for now. ;)

I'm very curious for the enigmatic review by yoroy (had to leave because of a return flight). Can anyone share it?

@tomgilissen, This is how I interpreted yoroy's comment: Since the field label already contains "recipient email address", lets not double this information from the field description.

Some comments on the content of the text:
- The original description does no contain the token "[user:mail]". It does not fall into the description of "to send the email to the author of the original post".
- The text "Available tokens are ..." implies that there is a limited set of tokens. This is not true because contrib modules may tokens too.
- The last sentence "Separate several recipients with a comma." can do without the word 'several'.

Two description suggestions. The minimal:

Alternatively, use tokens: [node:author:mail], [comment:author:mail], etc. Separate recipients with a comma.

The more verbose:
Alternatively, use tokens [node:author:mail], [comment:author:mail], etc. to send the email to the author of the original post. Separate recipients with a comma.

Sorry, had to catch the flight home soon after the enigmatics :)

Sutharsan's mimimal version looks great to me!

Sutharsan's second description suggestion sounds good to me.

I've uploaded a patch that follows the minimal suggestion by Sutharsan in #48 and supported by yoroy in #50: Improvement-help-text-Recipient-field-2603236-55.patch

Yep, looking good, thanks everybody!

Thanks! looks good.

Thanks everyone for working on this!

Note that the "after" screenshot in the summary does not reflect the current patch. I've removed it from the summary, because I almost posted a review based on the text in the screenshot, which was inaccurate.

+++ b/core/modules/action/src/Plugin/Action/EmailAction.php
@@ -164,10 +164,10 @@ public function defaultConfiguration() {
+      '#description' => t('Alternatively, use tokens: [node:author:mail], [comment:author:mail], etc. Separate recipients with a comma.'),

I think the word "Alternatively" is a bit odd here, because it's not being given as an alternate to anything. Maybe:

You may also use tokens: [...]

Changed the text as per suggested in #59 . Thanks

Committed 07f1e50 and pushed to 8.3.x. Thanks!