Update the link descriptions on the Configuration page for the system module

ifrik says we should credit: mairi, snehi, rachel_norfolk for this patch as well as herself/

So most of this looks great! I had just a couple of questions/comments:

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -40,14 +40,14 @@ system.admin_config_media:
   +  description: 'Set in which directories to store uploaded files, including translation files, and configure how they are accessed.'
   

   I'm not a huge fan of this wording. "Set in which..." is not a great way to start it out. Can we try to do better?

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   -  description: 'Configure display format strings for date and time.'
   +  description: 'Configure display formats for date and time.'
   

   This is especially good. I really don't think we should be using the word "strings" in user-facing help (as opposed to developer help). YES!

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -151,5 +151,5 @@ system.admin_reports:
   +  description: 'Get a status report about your site''s operation including any problems detected.'
   

   Does this need a comma? before including? Maybe not, I am sometimes comma-happy

Looks good to me!

Most of the improvements in this patch look good to me as well. Few questions:

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -56,7 +56,7 @@ system.admin_config_services:
   -  description: 'Configure the site description, the number of items per feed and whether feeds should be titles/teasers/full-text.'
   +  description: 'Configure a feed that contains your site''s content and that is located on the path <em>/rss.xml</em>.'
   

   I'm actually pretty sure these settings no longer have any impact on /rss.xml anymore at all, because it's a view. I tested and confirmed they seem to have no impact.

   IIRC we were keeping around the default number of items for one use in core that hadn't been converted to Views yet; not sure if it ever got converted or not. In any case this is a separate bug. Let's remove this hunk from the patch and file a followup.

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -128,19 +128,19 @@ system.admin_config_ui:
   -  description: 'Tools that enhance the user interface.'
   +  description: 'Enhance the user interface.'
   

   This is an... interesting one. I guess the updated description follows the standard better, but I find myself wondering what "enhance" means in this context. Like, why would anyone ever want a user interface that wasn't "enhanced"? So I think this one could be improved more.

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -128,19 +128,19 @@ system.admin_config_ui:
   -  description: 'Content workflow, editorial workflow tools.'
   +  description: 'Manage content workflow and editorial workflow.'
   

   How is editorial workflow different from content workflow?

I'd be okay with committing #2 and #3 as is and improving them further if desired in a followup, since this patch contains a lot of string improvements that would be good to get in ASAP. However, I think #1 is actually making an existing bug worse, so let's remove that at least.

@Xjm removing rss things.

Please review this one.

Apparently this needs to be "rc deadline" because it changes translatable UI text strings. See https://groups.drupal.org/node/484788

The latest patch does not seem to address *any* of @xjm's feedback. Did you just upload the previous patch by mistake?

Note that the followup exists already, see #2409413: Remove fields that do nothing from the "RSS publishing" settings form.

Thanks @snehi for looking at this issue so quickly.

+++ b/core/modules/system/system.links.menu.yml
@@ -56,7 +56,7 @@ system.admin_config_services:
-  description: 'Configure the site description, the number of items per feed and whether feeds should be titles/teasers/full-text.'
+  description: 'Configure a feed that contains your site''s content.'

This change should simply be removed from the patch. Once that's done, I'd be okay with it going in as an iterative improvement, and live with the other two strings for 8.0.x so that we can improve the others in time for translators to translate them. Of course, improving them would be great too if someone has time to do it today. ;)

I also pinged @Bojhan since this issue is tagged for usability review yet.

I wrote a long comment, but I'd like to remove the "enhance" and the "feed" ones. Both are misleading. The other changes can go in.

I also don't think "Select an image processing toolkit if additional toolkits are installed.'" is very good but its not worse what is there already.

changes as suggested by @xjm in #10.

Thanks! But we need additional changes (see #10 and #11):

a)

@@ -128,19 +127,19 @@ system.admin_config_ui:
   title: 'User interface'
   route_name: system.admin_config_ui
   parent: system.admin_config
-  description: 'Tools that enhance the user interface.'
+  description: 'Enhance the user interface.'

Revert this change -- leave this change as it was, I guess? Note, Bojhan: that word "enhance" is there already. The change here is just to make it start with a verb so that the descriptions on the page all do that...

Maybe we could change it to "Modify and configure the user interface"?

Although... This is one of the categories/sections on the Configuration page. In reality, the headers for these re not even shown on the page.

Oh. If you have your toolbar menu vertical, the descriptions are shown as hover text over the words. So I guess it is relevant.

Anyway, let's see. What's in the "User Interface" section currently is just Shortcuts. So how about maybe changing this to:

Configure and modify the administrative user interface.

or something like that?

b)

 system.rss_feeds_settings:
   title: 'RSS publishing'
   parent: system.admin_config_services
-  description: 'Configure the site description, the number of items per feed and whether feeds should be titles/teasers/full-text.'
   route_name: system.rss_feeds_settings

This was a mistake/misunderstanding from #10 -- we do not want to *remove* this description line from the file, we just don't want to change the line in the patch.

c) Regarding the image toolkit... I don't think the new or old text is really preferable, so let's just not make a change in the patch (it will mean more work for translators without a clear reason to make the change).

@jhodgdon, @xjm and @Bojhan Thanks for guiding us for the patch and patience.
We are not able to think what have written.
Anyway Please see attached diff and patch. I hope we are making sense this time.

I'm sorry, but this patch missed the Release Candidate deadline. At this point, we will have to delay the patch until Drupal version 8.1.x I believe.

Some of these look like they could be more concise to me - notes below.

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -40,7 +40,7 @@ system.admin_config_media:
   +  description: 'Configure where to store uploaded files, including translation files, and how they are accessed.'
   

   What about:

   'Configure the location of...'?

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure display formats for date and time.'
   

   'Configure date and time display formats'?

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure general system settings.'
   

   Configure...settings isn't ideal. What's under here?

4. +++ b/core/modules/system/system.links.menu.yml
   @@ -128,19 +128,19 @@ system.admin_config_ui:
   +  description: 'Manage content workflow and editorial workflow.'
   

   What's the difference between content workflow and editorial workflow?

5. +++ b/core/modules/system/system.links.menu.yml
   @@ -128,19 +128,19 @@ system.admin_config_ui:
   +  description: 'Configure settings related to formatting and authoring content.'
   

   Same with 'Configure ... settings'. What about just dropping settings here?

patch uploaded for #19. Please review.

Um. Please try again. Looking at the interdiff:

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -40,7 +40,7 @@
   +  description: 'Configure the location of where to store uploaded files, including translation files, and how they are accessed.'
   

   Still redundant. "where to store" can be removed.

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -110,7 +110,7 @@
   -  description: 'Configure general system settings.'
   +  description: 'Configure system settings.'
   

   Did you read comment #19, item 3? He asked for this to describe what the general settings are.This change doesn't help resolve the problem, and in fact makes it worse by taking out the word "general" which at least gave us some kind of an idea of what the settings are. Now there is even less idea.

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -134,13 +134,13 @@
   -  description: 'Manage content workflow and editorial workflow.'
   +  description: 'Manage the contents of editorial workflow.'
   

   This change is wrong... it's not "contents of editorial workflow" at all. Probably should just be "Manage content workflow".

4. +++ b/core/modules/system/system.links.menu.yml
   @@ -134,13 +134,13 @@
   +  description: 'Configure formatting and authoring content.'
   

   I don't think I like the structure of "Configure formatting". It seems very awkward. Can this be reworded? Maybe "Configure content formatting and authoring"?

thanks jhodgdon, the patch updated. Please review.

+++ b/core/modules/system/system.links.menu.yml
@@ -91,26 +91,26 @@ system.admin_config_regional:
-  description: 'General system related configuration.'
+  description: 'Configure site information, actions and cron settings.'

Now its clear that what information under that.

good to go as per comments in #22 and #19. +1 for RTBC.

Good! The only thing I see now is very minor: we use serial commas in the Drupal project, so in "Configure site information, actions and cron settings.", we need a comma before "and". Thanks!

Changes updated, please review.

Looks good to me now, thanks! These seem much better than what is in Core currently.

I am tagging this for Usability team review before setting to RTBC however.

Thanks, lots of improvements in here already but have a few more nitpicks. I think in general some of the descriptions try to be too complete:

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -40,7 +40,7 @@ system.admin_config_media:
   +  description: 'Configure the location of uploaded files, including translation files, and how they are accessed.'
   

   Why single out the translation files? "Configure the location of uploaded files and how they are accessed."

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -67,19 +67,19 @@ system.admin_config_development:
   +  description: 'Take the site offline or bring it back online.'
   

   Maybe use the "update" word here because that's one of the main scenarios for when you want to use this. "Take the site offline for updates and other maintance tasks."

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -67,19 +67,19 @@ system.admin_config_development:
   +  description: 'Configure the logging of errors and the display of error messages.'
   

   Much better! Does this only apply to errors though?

4. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Set a default country and timezone, and whether users can choose their own timezones.'
   

   "Settings for timezone and country." Maybe this is enough, we don't have to spell out each option that's available behind a link.

5. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure date and time display formats.'
   

   "Configure how dates and times are displayed." Maybe a bit more straight forward?

6. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure local site search, metadata, and SEO.'
   

   Why "local"?

7. +++ b/core/modules/system/system.links.menu.yml
   @@ -128,19 +128,19 @@ system.admin_config_ui:
   +  description: 'Configure and modify the administrative user interface.'
   

   Are configure and modify so different from eachother that we have to use both words?

I applied most of these perfect suggestions from #29 except these ones :

• For (3), I made a change with
  

  Configure the display of error messages and the database logging.

  , because display of error messages appears first. Any thoughts ?

• For (4), I would replace country by locale
  

  Settings for timezone and locale.

  . Any thoughts ?

• For (7), I'm not sure this is correct ?
  

  Configure the administrative user interface.

+++ b/core/modules/system/system.links.menu.yml
@@ -67,7 +67,7 @@
+  description: 'Take the site offline for updates and other maintance tasks.'

Minor spell mistake here. If i am not wrong, it should be maintenance task.

Rest of the things are good as per the feedback in #29. Thanks @pguillard.

As a usability improvement only for user-facing strings, I think we can still make this change during the beta per the beta allowed changes, since the only disruption is to translations and strings are not frozen until RC. (If it is not in by the RC then we would move it back to 8.2.x.)

Thanks for working on this! Marking NW for #33.

ifrik says we should credit: mairi, snehi, rachel_norfolk for this patch as well as herself/

Can we have those contributors all comment on the issue so that credit can be granted? I see snehi has commented but not the others.

spell error in "maintenance"
changed according to #33
rest is the same.

#37 was missing a new line in end of file
Just applied this change and addded interdiff.

Thanks for moving this forward. Almost there! 3 more nitpicks found. None are essential but we should always try and make things work with fewer words :)

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -67,19 +67,19 @@ system.admin_config_development:
   +  description: 'Clear caches, configure page caching for anonymous users, and enable bandwidth optimization for CSS and JavaScript files.'
   

   Would "Settings for caching and bandwidht optimization for CSS and JavaScript files." be enough?

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -67,19 +67,19 @@ system.admin_config_development:
   +  description: 'Configure the display of error messages and the database logging.'
   

   Minor nitpick: that "the" in "the database logging" seems off to me. Can a native speaker have a look?

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -151,5 +151,5 @@ system.admin_reports:
   +  description: 'Get a status report about your site''s operation, including any problems detected.'
   

   Do we have to call out the potential problems? I'd expect that from a status report. I think the "including any problems detected" could be removed

Agree with #42 suggestion, applied in this patch.
Concerning #2, I can't confirm as I'm not a native speaker..

+++ b/core/modules/system/system.links.menu.yml
@@ -67,19 +67,19 @@ system.admin_config_development:
+  description: 'Settings for caching and bandwidht optimization for CSS and JavaScript files.'

typo: bandwidth

After that typo fix I think we're good to go :)

Updated accorded to #47

Thank you!

Pretty good, but I don't think this is quite ready:

1. +++ b/core/modules/system/system.links.menu.yml
   @@ -67,19 +67,19 @@ system.admin_config_development:
   +  description: 'Settings for caching and bandwidth optimization for CSS and JavaScript files.'
   

   I don't like that most of the descriptions here start with verbs, such as "Configure the..." and "Take the site...". But this one starts with "Settings for".

   Can we make them all parallel?

   Also... The way this is written and punctuated, it reads to me as though it's saying that caching is only about CSS and JavaScript files.

   I think a comma after "caching" would help avoid that mis-interpretation.

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Settings for timezone and country.'
   

   Here's another one that doesn't start with a verb.

3. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure site search, metadata, and SEO.'
   

   I am not sure we should be using the SEO acronym in this description? Maybe write it out as "search engine optimization"... but really, are there any SEO settings in here anyway?

4. +++ b/core/modules/system/system.links.menu.yml
   @@ -91,26 +91,26 @@ system.admin_config_regional:
   +  description: 'Configure site information, actions, and cron settings.'
   

   What does "site information" mean?

1. @jhodgdon you don’t or you do like that most start with verbs? Because you’re asking to change the ones that don’t :)

2. Agreed, lets add, that comma :)

3. SEO is mentioned because URL aliases are listed here and SEO related contrib modules are supposed to add their links here.

4. "Site information" is the name of one of the items inside this category. Granted, it is very generic wording.

I've got another question; I don't see all descriptions in this patch being used, especially the ones for the section labels on the configuration page:

Are we adding unnecessary labels or am I looking in the wrong place or is this by design for Seven and Bartik but other themes may choose to show these descriptions?

1. Sorry! I like that most of them start with verbs. I don't like that a few don't start with verbs. Apologies for the confusion!

3. RE SEO -- I think that instead of an acronym like SEO it would be better to write out "search engine optimization".

4. RE "site information"...Yeah, the "Site information" page at admin/config/system/site-information has a lot of random settings on it, and I don't think that "Site information" is a good name for that page... it includes configuration for the site name, site slogan, site-wide email address, 403/404 pages, and home page. I think a name like "basic site settings" would make a lot more sense than "site information".

5. RE descriptions -- You are right that the section heading descriptions are not displayed on admin/config. However, if you make the main toolbar admin menu vertical, expand the Configuration menu item, and hover over the menu items under Configuration (such as the Search and Metadata item), you will see the descriptions there as tool tips.

Also, yes this is theme-dependent. The template for each section of the page is admin-block.html.twig. The docs say:

...
 *   - title: The block title.
 *   - content: (optional) The content of the block.
 *   - description: (optional) A description of the block.
 *     (Description should only be output if content is not available).

So according to the template (and its code), if the block has some links in it, you output the links. If it is empty of links, you output the description instead. That is what the default/stable templates do:

  {% if block.content %}
    <div class="panel__content">{{ block.content }}</div>
  {% elseif block.description %}
    <div class="panel__description">{{ block.description }}</div>
  {% endif %}

Some other admin theme would be free to override this and output the description as well.

Anyway... that is probably more information than you wanted, but the descriptions are definitely being used now in the vertical format of the main admin navigation menu anyway, as tooltips.

1. No worries, I figured that's what you meant but since we’re nitpicking words in here I had to double check :)

3. Agreed, lets spell it out.

4. I like "Basic site settings", this means we want to change the label of that link as well. Might make this a 8.2 thing, potentially breaking tests maybe?

5. Consider me sufficiently informed on the matter ;-) Good to know we do indeed use these descriptions now.

RE item 4 -- UI text is not frozen until the first release candidate, so I think we can go ahead and change the page name to "Basic site settings". We're changing other UI text in this patch anyway.

If we do so:

a) Tests may fail, but we can fix them in this patch.

b) We should check the System help page (function hook_help() in system.module) and see if it mentions this page by name. If so, the page name will need to be edited in the help. I suspect it is there.

Anyway, neither (a) nor (b) would be out of scope, I think, for this issue.

So.... All that taken into account, I think what this issue/patch needs is several fixes in the system.routing.yml and system.links.menu.yml files:

1. (links file) Fix the description for system.admin_config_development so that it (a) starts with a verb and (b) has a comma after "caching" on:

@@ -67,19 +67,19 @@ system.admin_config_development:
+  description: 'Settings for caching and bandwidth optimization for CSS and JavaScript files.'

2. (links file) Fix the description for system.admin_config_regional so that it starts with a verb.

3. (links file) Check once again and make sure all of the entries directly under admin/config start with verbs, if there are maybe more than those two that currently don't.

4. (links file) Replace SEO with "search engine optimization".

5. [links and routing files, I think? Plus system.module - function system_help()]: Change the name/link for the admin/config/system/site-information file to be "Basic site settings" instead of "Site information". Also change the description for system.admin_config_regional in the links file to say "basic site settings" instead of "site information". Also in system_help(), change any links to the page to use the new page name.

6. If tests fail after making those changes, especially item 5, fix the test failures.

Here's a new patch.

Fix test failures (somewhat expected).

It's working fine.

This looks like a major improvement, RTBC for me.

Maybe 2 more small improvements :

1. "Configure default user account settings, including fields, registration requirements, cancellation rules, and email messages."

2. +++ b/core/modules/system/system.links.menu.yml
   @@ -56,64 +56,64 @@ system.admin_config_services:
   +  description: 'Configure the timezone and country.'

   I would suggest "Configure the locale and timezone settings." to litterally tell what we find there.

Those suggestions look good. However, this is about the System module and the user account settings description is in the user module, so it is out of scope for this issue. So, fixed the other one one.

@ktdimuthu, not sure why you hid those files, please don't do that -- they were the current files!

Anyway, here's a new patch.

No one ? then sold, RTBC !

+++ b/core/modules/config/src/Tests/ConfigFormOverrideTest.php
@@ -35,7 +35,7 @@ public function testFormsWithOverrides() {
+    $this->assertTitle('Basic site settings | ' . $overridden_name);

Opened #2686691: Move custom 403/404 settings to error handling to move custom 403/404 pages out of this. It's a bit confusing to me why they're in there apart from being that way since 4.5 or whenever.

Otherwise looks great to me, so committed/pushed to 8.2.x and 8.1.x, thanks!