Formatter summaries need to return a string, but it would be nicer if they returned an array and then Field UI does the implode().

Comments

yched’s picture

Agreed.
@sun even advocated for a render array.

swentel’s picture

Issue tags: +Field API

tagging

swentel’s picture

Status: Active » Closed (fixed)
swentel’s picture

Assigned: swentel » Unassigned
Status: Closed (fixed) » Closed (won't fix)
Issue tags: -Field API
swentel’s picture

Project: D8 Field API » Drupal core
Version: » 8.x-dev
Component: Code » field_ui.module
Assigned: Unassigned » swentel
Status: Closed (won't fix) » Active
Issue tags: +Field API

Ok, nevermind, let's make sure the patches are reviewable. Moving to core queue

swentel’s picture

Status: Active » Needs review
StatusFileSize
new24.42 KB

While, we're there, cleanup docs as well.

Status: Needs review » Needs work

The last submitted patch, 1854868-6.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
StatusFileSize
new735 bytes
new25.13 KB

Of course you need to fix the tests too ...

yched’s picture

Minor:

+++ b/core/modules/field/tests/modules/field_test/field_test.moduleundefined
@@ -272,7 +272,8 @@ function field_test_field_formatter_settings_form_alter(&$element, &$form_state,
+  $summary[] = '<br />field_test_field_formatter_settings_summary_alter';

The <br /> is not needed anymore ?

Other than that, looks good to me :-)

swentel’s picture

StatusFileSize
new725 bytes
new25.13 KB

Oh sweet irony .. :)

yched’s picture

Status: Needs review » Reviewed & tested by the community

Yay, thanks !

aspilicious’s picture

Great!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/field_ui/field_ui.api.phpundefined
@@ -93,10 +93,11 @@ function hook_field_instance_settings_form($field, $instance, $form_state) {
 /**
- * Alter the formatter settings form.
+ * Alter the formatter settings form invoked from
+ * \Drupal\field_ui\DisplayOverView.

Like this..

/**
 * Alters the formatter settings form.
...
...
 *
 * @see \Drupal\field_ui\DisplayOverView

And do the same for the other similar cases...

swentel’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new1.69 KB
new25.76 KB

Done

alexpott’s picture

Title: Let formatter summaries return arrays instead of strings » Change notice: Let formatter summaries return arrays instead of strings
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed 30e1599 and pushed to 8.x. Thanks!

swentel’s picture

Status: Active » Needs review

Change notice created at http://drupal.org/node/2003680

aspilicious’s picture

Priority: Critical » Normal
Status: Needs review » Fixed

Excellent, we don't need more!

amateescu’s picture

Title: Change notice: Let formatter summaries return arrays instead of strings » Let formatter summaries return arrays instead of strings
Issue tags: -Needs change record

Reviewed and refined the change notice on irc, looks good to me now.

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