Problem/Motivation

In #2829283: Language support in Feed import it appeared there is need for returning multiple lines in the summary. I checked how core handles this and it appears that \Drupal\Core\Field\FormatterInterface::settingsSummary() already handles this and returns an array.
The Field UI module than renders this summary in Drupal\field_ui\Form\EntityDisplayFormBase::buildFieldRow():

// Display a summary of the current plugin settings, and (if the
// summary is not empty) a button to edit them.
$summary = $plugin->settingsSummary();

// Allow other modules to alter the summary.
$this->alterSettingsSummary($summary, $plugin, $field_definition);

if (!empty($summary)) {
  $field_row['settings_summary'] = [
    '#type' => 'inline_template',
    '#template' => '<div class="field-plugin-summary">{{ summary|safe_join("<br />") }}</div>',
    '#context' => ['summary' => $summary],
    '#cell_attributes' => ['class' => ['field-plugin-summary-cell']],
  ];
}

Proposed resolution

To support multiple summary lines, let's allow ConfigurableTargetInterface::getSummary() to return an array. MappingForm::buildRow() can then use the same technique as what the Field UI module does.

This is an API change, but it makes the patch for the multilingual issue slightly simpler, so that's worth it in my opinion.

Remaining tasks

  • Review
  • Commit

User interface changes

None (I hope).

API changes

Interface Drupal\feeds\Plugin\Type\Target\ConfigurableTargetInterface changes: it's method ::getSummary() may now also return arrays. In fact, that becomes the recommended way.

Data model changes

None.

Patch will follow.

Comments

MegaChriz created an issue. See original summary.

megachriz’s picture

Status: Active » Needs review
StatusFileSize
new7.97 KB

Let's see if this patch breaks any tests.

megachriz’s picture

StatusFileSize
new5.79 KB

Oops, forgot to rebase. Patch in #2 undoes the latest commit in dev.

The last submitted patch, 2: feeds-summary-as-array-3113960-2.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 3: feeds-summary-as-array-3113960-3.patch, failed testing. View results

megachriz’s picture

Status: Needs work » Needs review
StatusFileSize
new5.85 KB
new588 bytes

Fixing TextTest.

  • MegaChriz committed c84c91e on 8.x-3.x
    Issue #3113960 by MegaChriz: Allow ConfigurableTargetInterface::...
megachriz’s picture

Status: Needs review » Fixed

I tested the patch from #6 in the UI with several targets. There was in fact one target summary getting double escaped, but that target did not come from this module. To be more specific: that target came from the patch in #2933361: Add a role mapping target.

Committed #6.

Status: Fixed » Closed (fixed)

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