Problem/Motivation

From #2810871: Explain fields configuration and private discussion with @miro_dietiker, we still need to improve some wording for the Diff settings UI.
Text/UI wording is never easy to define, thus we need to discuss properly to give to the user better/short/clean/concise descriptions.

From the parent issue (comment #21):
1.

Yeah, these "per category settings details-wrappings" could be easier for the end user/self-explaining, but then for the visual_inline_theme aren't the label "VISUAL INLINE LAYOUT SETTINGS" and the description "Theme used for Visual inline layout when comparing revisions." too much/repetitive ?
Not sure, but imo, if we would keep it like this, I'd suggest removing the description (or at least saying something more "useful", like the Leading/Trailing examples ?)

2.

About "Leading context lines", "Trailing context lines" and their descriptions, not sure which better wording we can use to not repeat things. Maybe just "Leading" and "Trailing" as select labels and keep the descriptions (?)

3.

More:
+++ b/diff.module
@@ -78,11 +78,14 @@ function diff_help($route_name, RouteMatchInterface $route_match) {
+      return '<p>' . t('This table provides a summary of the field support found on the system. For every field a diff plugin can be selected and configured. This settings are applied to Unified and Split fields layouts.') . '</p>';

As you said once "We want to avoid the diff word in the UI" (comment #15.2), how can we change this "For every field, a diff plugin can be selected and configured." ?

Also imo this new help text is too long, but if it is fine, then: "This" --> "These".

+++ b/src/Form/GeneralSettingsForm.php
@@ -98,11 +82,12 @@ class GeneralSettingsForm extends ConfigFormBase {
+

Unnecessary new line.

+++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -25,6 +25,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
+ *   description = @Translation("Visual layout, revision comparison is made as the content is rendered for the user."),

Not sure what you mean by "made as the content is rendered for the user".

Private chat:

[...]
the (visual inline) plugin triggers an entity view / rendering.
instead of field based view / rendering / value comparision.
The user usually doesn't know the term rendering though.
Dunno, it's an admin screen, so we might be fine to use those terms.

It might be healthy to do it there. We found the word "Visual inline" for the end user, but in the plugin description, it's the time to explain what it means..

I was even thinking about removing "Inline" from the plugin label because "Visual" might be clear enough.

Proposed resolution

- discuss about the best wording to display to the end user/site builder
- upload the patch
- review

Remaining tasks

User interface changes

Improved some wording in /admin/config/content/diff/general and /admin/config/content/diff/fields

Before:

After:

API changes

Data model changes

Comments

tduong created an issue. See original summary.

tduong’s picture

Assigned: Unassigned » tduong
Status: Active » Needs review
Parent issue: » #2810871: Explain fields configuration
StatusFileSize
new9.55 KB
new126 KB
new126.42 KB

Provided some suggestions for most of the points mentioned in the IS, but still the diff word is present in the field_list and I've just noticed also the general_settings help texts. Still needs discussion.

About the Visual layout settings detail form, I've tried to use an ajax to display the selected theme description, but somehow when I switch to admin the description is not changed, I guess I'm missing something. Any suggestions how to fix it? Do you approve it?

I've provided a description for the Visual plugin, hope this makes sense. I've seen that entity type is also used in the diff.field_list page, and view mode is also used in the entity type edit page, so I thought these words are fine for the end user (?)

If we decide to actually remove "inline" from the "Visual inline" plugin label, this needs more work (find where visual_inline is used and replace it with jsut visual, ...)

tduong’s picture

Title: Provide better description wording for the Diff settings UI. » Provide better description wording for the Diff settings UI
Issue summary: View changes
StatusFileSize
new131.2 KB
new3.79 KB
new7.07 KB

Shortly discussed with @Berdir about the Visual layout settings form, and a static description which explains what happens when one or the other option is selected is better than what I've tried to do. Reverted ajax and themeHandler changes and provided a static description.

johnchque’s picture

+++ b/src/Form/GeneralSettingsForm.php
@@ -169,14 +168,20 @@ class GeneralSettingsForm extends ConfigFormBase {
-          'standard' => $this->t('Standard'),
+          'default' => $this->t('Standard'),

Not really sure about this change, then the option label should be also "Default"?

Status: Needs review » Needs work

The last submitted patch, 3: provide_better_description_wordings-2817159-3.patch, failed testing.

johnchque’s picture

Status: Needs work » Needs review

Unrelated change. Will be fixed in #2817185: Fix test fail in HEAD

tduong’s picture

True, I needed it before to avoid adding more checks just to change from standard to default and get the related description.
Reverted it.

What about the other touched wordings ?

johnchque’s picture

Status: Needs review » Needs work
  1. +++ b/config/install/diff.settings.yml
    @@ -13,4 +13,4 @@ general_settings:
    -  visual_inline_theme: standard
    +  visual_inline_theme: default
    

    then this change is not needed either. :)

  2. +++ b/src/Form/GeneralSettingsForm.php
    @@ -169,6 +168,9 @@ class GeneralSettingsForm extends ConfigFormBase {
    +      $default_theme_name = $this->config('system.theme')->get('default');
    
    @@ -176,7 +178,10 @@ class GeneralSettingsForm extends ConfigFormBase {
    +          '%default' => $default_theme_name,
    

    we want to be consistent here, let's use standard here too then.

Besides that looks good.

tduong’s picture

Status: Needs work » Needs review
StatusFileSize
new230 bytes
new6.73 KB

Leftover fixed. Thanks :)

About the second one, I think that it's fine because the theme actually is called default. "Standard" was a hardcode "label" suggested once by @Berdir for an issue (#2799597: Select theme for diff display, comment #24.2).

miro_dietiker’s picture

Status: Needs review » Needs work

IMHO we should list them with their name. It's not an implicit assignment. It's an

+++ b/src/Form/GeneralSettingsForm.php
@@ -176,7 +178,10 @@ class GeneralSettingsForm extends ConfigFormBase {
+        '#description' => $this->t('Use Standard to display the comparison as %default theme, or Admin as %admin theme.', [
+          '%default' => $default_theme_name,
+          '%admin' => $admin_theme_name,

admin/appearance clearly talks of default and admin theme. It doesn't use the term "standard" so we should stay with "default".

miro_dietiker’s picture

Status: Needs work » Needs review
StatusFileSize
new8.67 KB
new2.78 KB

Renaming the settings default.

  • miro_dietiker committed bcc2a09 on 8.x-1.x authored by tduong
    Issue #2817159 by tduong, miro_dietiker, yongt9412: Provide better...
miro_dietiker’s picture

Status: Needs review » Fixed

Committed. :-)

tduong’s picture

Status: Fixed » Needs review

Good point :)

What about renaming "Visual inline" in just "Visual" topic ?

johnchque’s picture

I would vote for keeping it as "Visual Inline".

miro_dietiker’s picture

Status: Needs review » Fixed

If you want to discuss this, open a new issue. :-)

tduong’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.