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.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | interdiff-3113960-3-6.txt | 588 bytes | megachriz |
| #6 | feeds-summary-as-array-3113960-6.patch | 5.85 KB | megachriz |
Comments
Comment #2
megachrizLet's see if this patch breaks any tests.
Comment #3
megachrizOops, forgot to rebase. Patch in #2 undoes the latest commit in dev.
Comment #6
megachrizFixing TextTest.
Comment #8
megachrizI 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.