Improve search settings help text

Many of these items have been fixed, and some are duplicates of other issues. At this point, unless we have serious errors in UI, the UI is fixed for 7.x, so I am just going to close this as a "won't fix".

I think Xano still has a point here in the original report:

The "Indexing settings" fieldset contains italic text which should not be italic at all, but set as #description and it should not explain the default values are probably the best to use, since that is mostly the case with default values.

The attached patch:

1. Removes <p><em> markup from the help string. If there is a need for emphasized text, let's figure out how to pass message importance classes to the form elements.
2. Removes unneccessary "The default settings should be appropriate for the majority of sites." from the help text.

PS: It would be nice for fieldgroups to support #description property natively and re-use its styling.

Applied patch manually (2 lines were removed from search.admin.inc since patch 2) and the result was the same as the sample images. Ran all the search module tests and got 2080 passes and 0 fails.

patch with 2-line difference attached.

One minor thing is we should do t("...") to avoid having to escale the backslash in won\'t, so needs work for that.

I'm also wondering the rationale of removing that sentence about the defaults being fine. If I were confronted with those settings, I'd literally have no idea what to change them to, so that sentence actually helps me breathe a sigh of relief that I can safely ignore them unless I employ someone who used to work at Google. :P

Applied the patch from 6 and confirmed the change of language in the interface.

@webchick, while I also appreciate the reassuring-ness of the sentence we removed, I'm not sure conceptually that it fits. Drupal should ship with reasonable defaults for all configuration options. If the minimum word length and CJK handling really are appropriate for the vast majority of sites, it might be a better idea for them not be options at all and rely on contrib to override them. Or, changing them could trigger confirm_form()?

@Xano - please don't RTBC your own patch.

I'm not sure I agree with the sentence removal either, but while we're at it, how about we fix the other bit of help text that doesn't display at all, and if it did display it would also be wrapped in <em> tags as well.

Before:

After:

Not sure how I feel about adding <p> tags but without them the text ends up uncomfortably close to the table in Seven:

After reviewing the overlapping changes from #2572689, I've rolled a new patch against 8.2.x with the spirit of this issue:

• Removed the contraction and switched to single quotes
• Removed the duplicate "cron maintenance tasks" descriptions for indexing run information
• Moved the ['info'] markup into the fieldset description and took out the p and em tags
• Took out wording that explains what 'default' means, or mentions that the default search index is the default search index

Did cursory runs of phpunit and simpletest, and none of the errors were related to the admin page text.

See #2211241-33: Refactor search_reindex() into separate functions for introduction of new text that I subsequently removed with this patch.

Patch #13 shows error when applying.Needs reroll

Checking patch core/modules/search/src/SearchPageListBuilder.php...
error: while searching for:
    );
    $form['indexing_throttle']['cron_limit'] = array(
      '#type' => 'select',
      '#title' => $this->t('Number of items to index per cron run'),
      '#default_value' => $search_settings->get('index.cron_limit'),
      '#options' => $items,
      '#description' => $this->t('The maximum number of items indexed in each run of the <a href=":cron">cron maintenance task</a>. If necessary, reduce the number of items to prevent timeouts and memory errors while indexing. Some search page types may have their own setting for this.', array(':cron' => \Drupal::url('system.cron_settings'))),
    );
    // Indexing settings:
    $form['indexing_settings'] = array(
      '#type' => 'details',
      '#title' => $this->t('Default indexing settings'),
      '#open' => TRUE,
    );
    $form['indexing_settings']['info'] = array(
      '#markup' => $this->t("<p>Search pages that use an index may use the default index provided by the Search module, or they may use a different indexing mechanism. These settings are for the default index. <em>Changing these settings will cause the default search index to be rebuilt to reflect the new settings. Searching will continue to work, based on the existing index, but new content won't be indexed until all existing content has been re-indexed.</em></p><p><em>The default settings should be appropriate for the majority of sites.</em></p>")
    );
    $form['indexing_settings']['minimum_word_size'] = array(
      '#type' => 'number',

error: patch failed: core/modules/search/src/SearchPageListBuilder.php:203
error: core/modules/search/src/SearchPageListBuilder.php: patch does not apply

Rerolled patch #13.Please check

I reviewed the patch reroll and it looks good to me. Thanks for doing that, @abhijith-s !

Patch #22 applied successfully and it's working as expected.

Thanks for the interest but this still needs usability review.

Still waiting for UX review. But looking again at the issue summary not clear what is being updated. Think the issue summary should be updated for what the test changes are and why. Screenshots couldn't hurt.

@eric@erubino.net:

Thanks for the updated screenshots and for rewriting the issue summary.

I am making some further updates: looking at the patch from #22, I do not think it makes any changes to the table. And I want to be a little more specific about where the text is updated.

I think the biggest change is replacing the text

Search pages that use an index may use the default index provided by the Search module, or they may use a different indexing mechanism. These settings are for the default index. Changing these settings will cause the default search index to be rebuilt to reflect the new settings. Searching will continue to work, based on the existing index, but new content won't be indexed until all existing content has been re-indexed.

The default settings should be appropriate for the majority of sites.

with

Changing these settings will cause the default search index to be rebuilt to reflect the new settings. Searching will continue to work, based on the existing index, but new content will not be indexed until all existing content has been re-indexed.

I have to think a bit about whether we will miss any of the deleted text.

Usability review

We discussed this issue at #3376176: Drupal Usability Meeting 2023-07-28. That issue has a link to a recording of the meeting. For the record, the attendees at the usability meeting were @AaronMcHale, @Emma Horrell, @benjifisher, @lukas.fischer, and @rkoller.

We agreed that the proposed changes are an improvement. We especially considered the text that is removed (as I pointed out in #38) and we decided that removing it is a good idea.

I already updated the issue description when I added #38, so I am removing the tag for an update.

#22 still applies for 11.x

So believe this can be moved to RTBC

I'm triaging RTBC issues.

I read the issue summary and the comments. Thank you @eric@erubino.net for the issue summary update, it has made this task much easier. Also, thanks to @smustgrave for asking that it was done. Although, I do think that item 3 in the UI changes "content ranking is now sortable" is incorrect.

There has been a Usability review confirming that changes are OK. It is unfortunate that the patch is re-testing on 9.5 but this has been recently tested on 11.x. I have not found anything else that needs to be done here.

Leaving at RTBC.

Saving credits, and attributing usability review meeting participants

I reviewed the changes carefully with a word diff, and manually tested the page with the patch applied.

When I first reviewed the diff, I was concerned about losing any reference to cron. (Site owners often don't realize that they need to configure cron, so referencing it is important.) I was particularly concerned about losing the link to the cron page. However, when manually testing, I see that the cron link is still mentioned at the top of the page. It's still clear in context, so that's okay.

Overall I think the tradeoff is worthwhile. I'm linking a couple related UX issues for this and related admin pages that would make good followups.

As a set of string changes, this issue is minor-version only, so I committed it to 11.x and did not backport. Thanks everyone!