Wrong More link aria-label token [node:title] on non-nodes

I'm not sure if it's possible for the formatter to find out the entity type when it's being created. If it is, then I think the best way forward would to have a lookup table with the tokens for known entity types, and a sensible default for unknown ones.

However, if it's not possible to determine the entity type at creation, I think the best we can do is come up with a better default.

This was discussed with various community members during DrupalEasy office hours today.

Our consensus is to proceed as follows:

1. We can get the entity type in SmartTrimFormatter via $this->fieldDefinition->getTargetEntityTypeId()
2. We can then set more reasonable default values for the aria-label element based on the entity type name.
3. We should limit the default values to only core entity types (nodes, terms, users, custom blocks, comments, media entities) - we can use a switch statement to set the proper token in the default value.
4. We should add a bit more to the descriptive text of the aria-label field in the formatter's settings form to provide additional examples.
5. We should add a new functional test along with this change.

Thoughts?

-mike

I spent some time trying to get this working, but there's a few issues:

SmartTrimFormatter::defaultSettings() is a static function, therefore, $this is not available. Therefore, $this->fieldDefinition->getTargetEntityTypeId() is not available, therefore we cannot customize the aria-label default value there. ☹️

I went deep into trying to figure out where I might be able to put the aria-label default value customization code - but another issue is that the $settings array gets set in __construct() (via FormatterBase), but those values come from SmartTrimFormatter::defaultSettings().

So, by the time either SmartTrimFormatter->settingsSummary() or SmartTrimFormatter->settingsForm() gets called, $settings is already set. We cannot insert our code at this point, because we have no way of knowing if the values in $settings has been "selected" by the site-builder, or if the values are indeed, default.

I'd love for someone else to take a swing at figuring out if I missed anything and if there's a reasonable place to insert the necessary customization code.

Here's the code snippet I was going to use for the actual customization - it just needs a home!

$entity_type = $this->fieldDefinition->getTargetEntityTypeId();
    switch ($entity_type) {
      case 'node':
        $token = '[node:title]';
        break;

      case 'taxonomy_term':
        $token = '[taxonomy_term:name]';
        break;

      case 'block_content':
        $token = '[block_content:info]';
        break;

      case 'comment':
        $token = '[comment:subject]';
        break;

      case 'file':
        $token = '[file:filename]';
        break;

      case 'media':
        $token = '[media:name]';
        break;

      case 'user':
        $token = '[user:name]';
        break;

      default:
        $token = NULL;
    }
    if (!is_null($token)) {
      $default_settings['more']['aria_label'] .= ' ' . $this->t('about') . ' ' . $token;
    }

I also modified the defaultSettings() with 'aria_label' => 'Read more', (I removed the about [node:title] bit.)

-mike

Could we add a new property to the schema, called something like "initializing", which the SmartTrimFormatter::defaultSettings would set to true (it should be defined to allow nulls to avoid breaking backwards compatibility).

Then SmartTrimFormatter::settingsForm could check for that setting. If it is not present or false, the settings form would be loaded with the saved settings as normal.

However, if it is set to true, the aria label textbox could be replaced with the switch() statement above.

By the way, if we didn't need to support PHP 7.4, the new match() statement would be ideal for this. The above switch() could be reduced to:

$token = match ($entity_type) {
  'node' => '[node:title]',
  'taxonomy_term' => '[taxonomy_term:name]',
  'block_content' => '[block_content:info]',
  'comment' => '[comment:subject]',
  'file' => '[file:filename]',
  'media' => '[media:name]',
  'user' => '[user:name]';
  default => NULL,
};

I tried adding a new "initializing" field to the default settings. However, as there is no submit function, it needs to be set through the form fields. I thought using a "#hidden" field would cause it to update, but it didn't seem to.

So I switched to what seems a simpler approach of simply having a really unlikely default value for the Aria label. I went with "!DEFAULT!". If the settings form finds this value, it replaces it with a label tailored for the entity type.

I've tested for user, and it correctly set the Aria label to "Read more about [user:name]".

Incidentally, I think I might have found a bug in core, as when the Smart Trim settings appear in the Configuration->People->Account->Manage Display, the More Link settings are always shown, and checking/unchecking the "Display More Link" checkbox does not affect the display.

If this approach is acceptable, we'll need some tests. I think all of the current tests set the Smart Trim settings through $display_repository->getViewDisplay()->setComponent(), which bypasses the settings form, so I think we'd need some tests that test setting the Smart Trim settings through the form sets up the correct defaults, probably for a couple of entity types. However, I'd like some opinions on the approach before writing tests.

Looks like I never set the is "needs review".

I'm fine with this approach (using !DEFAULT!).

I think the proposed logic for setting the default aria label is sound

'#default_value' => ($more_settings['aria_label'] ?? NULL) == '!DEFAULT!' ? $this->getDefaultAriaLabel() : ($more_settings['aria_label'] ?? $this->getDefaultAriaLabel()),

I've been trying to think of a case where !DEFAULT! ends up in the response, but I think it is good.

Would it also make sense to add some additional options for a limited set of well-used contrib entities to the getEntityTypeToken() method? I'm thinking Commerce product names for one.

-mike