Remove thunder processors without entityreferences; extend tests.
Closes #3447677
Merge request reports
Activity
377 377 378 378 // When the plugin provides a form, show an edit button. 379 379 // But actually hide the form for now. 380 if (!empty($field_row['plugin']['settings_edit_form']['form']['settings'])) { 380 if (!empty($field_row['plugin']['settings_edit_form']['form'])) { -
Un-bug this, so that you at least can see the form if you want to. Balint's #3447556 will conflict with this but he can solve it.
43 // Make single-value fields flat by default. 44 if ($field_items->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() == 1) { 45 $values = reset($values); 48 if (empty($this->configuration['settings']['forceSingleValue']) 49 && $field_items->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() != 1) { 50 // Setting a full a multi-value array (likely with multiple properties 51 // per value) into a single property is possible, but obviously puts the 52 // burden on the 'client' to decode this. 53 $values = $field_items->getValue(); 54 if ($is_slot) { 55 $custom_element->addSlot($element_name, is_scalar($values) || $values instanceof MarkupInterface ? $values : Json::encode($values)); 56 } 57 else { 58 $custom_element->setAttribute($element_name, $values); 59 } 60 return; -
So, moving the preexisting code into this if().
Thought about it a bit. My current conclusion: I guess people may still want to do this, either in a property or in a slot depending on their decoding needs, so hey why not. Let's just output a big array and decide that the RawCeFieldFormatter doesn't want special behavior for multi-value fields (like putting each value into a slot or whatever).
changed this line in version 6 of the diff
85 } 86 $custom_element->setAttribute($name, $property->getValue()); 87 } 52 88 } 53 89 } 54 90 91 /** 92 * {@inheritdoc} 93 */ 94 public function defaultConfiguration() { 95 return parent::defaultConfiguration() + [ 96 'settings' => [ 97 'forceSingleValue' => FALSE, 98 ], 99 ]; 100 } -
@fago I'm trailing behind you like always. Would you say
- this checkbox in the form is 'formatter settings' and should therefore in configuration][settings?
- or configuration][settings is almost-exclusively meant for 'third party' things like Core field formatters and we just want to put our formatter options directly inside 'configuration' (just like your /EntityReferenceCeFieldFormatter does, I see)
I better get this right, now -- but I don't know how to judge this. I just realized that in the first case, I don't really know a use case for properties directly underneath 'configuration'... (See the link export below, for how values are exported now.)
-
For completeness:
- I know this isn't the most urgent thing right now, but my main aim is to understand the intended situation. Then I can fix it sometime before beta.
- If 'is_slot'+'name' are supposed to be underneath 'configuration', instead of next to 'configuration'+'formatter'... then I understand.
- This is also kind-of suggested by https://www.drupal.org/project/custom_elements/issues/3445143 last box which says
component-of-a-formatter: formatter: <formatter-id> configuration: settings: <array-of-formatter-settings> custom-config: <value>- However, then the config schema needs to be modified.
- And then I'm still not 100% sure if we want 'forceSingleValue' beneath configuration][settings. Probably?
- I have now several times seen duplicate 'is_slot' in exported config, like in the below example. The outer is_slot is the 'active' value, on both import+export. I'll create a followup for that, when I understand what needs to be done.
content: field_media: formatter: entity_ce_render name: media is_slot: true weight: 0 region: content configuration: is_slot: false mode: full hidden:Edited by Roderik Muit changed this line in version 6 of the diff
added 1 commit
- c4db70e1 - Get field definition from the plugin itself.
68 // "<configured name>-<own name>". 69 $field_item = $field_items->first(); 70 $main_property_name = $this->getFieldDefinition()->getFieldStorageDefinition()->getMainPropertyName(); 71 $main_property = $field_item->get($main_property_name); 72 if ($main_property instanceof PrimitiveInterface) { 73 if ($this->isSlot()) { 74 $custom_element->setSlot('default', $main_property->getValue()); 75 $custom_element->setAttribute($element_name ?: $main_property_name, NULL); 76 } 77 else { 78 $custom_element->setAttribute($element_name, $main_property->getValue()); 79 } 49 80 } 50 else { 51 $custom_element->setAttribute($element_name, $values); 81 foreach ($field_item->getProperties(TRUE) as $name => $property) { -
I don't think we should do this. This is not "Auto" formatting, so it should not apply any "Auto"-logic but be dumb and only apply the settings. If that makes sense or not, it's up to the users, but you should get what configure.
--> I'd expect forceSingleValue to behave like the regular raw formatter, with only one difference: Flatten the array of a single field-item from being one with multiple-properties, to be a single value (primitive or not) of the main-property. If it's multi-valued, it should be an array of single-values (instead of an array of field-item-values). That's it.
Any other logik like de-flattening things into e.g. "
name" is imo out of scope here and not what I'd expect to be triggered by this setting. If anything, I'd suggest to make a dedicated formatter or option for it. but let's not do this for now, we can add this feature later. -
OK, I understand your reasoning that the raw formatter should not do any processing. (Though that would mean that a property for the most simple field value to be output in custom-elements HTML for any regular Drupal field, looks like
NAME="{"value":"VALUE"}".)But we cannot "let's not do this (de-flattening) for now".
You cannot expect me to accept that
<pg-link type="link" view-mode="full" link="{"uri":"http:\/\/example.com","title":"Example site","options":[]}"></pg-link>is the only possible option, i.e. that<pg-link type="link" view-mode="full" link="http://example.com" title="Example site"></pg-link>is just not possible in the standard module, and then not be horribly confused. (Not to mention other users of the contrib module.)I need the latter form, to keep my sanity while making everything else downstream from here, work. It affects almost every piece of output.
I don't care if it's an option or a new formatter (called "Simple properties"). You're welcome to decide on that. For the moment, I'll make it an option, because that's the quickest -- so I can continue on the rest.
Edited by Roderik Muit -
@roderik ok, thanks. I take your input as reason for "we really need this complexity".
I think I'd prefer to have it in a separate plugin, which can extend the Raw one. What about calling it: "Flattened" ?
changed this line in version 6 of the diff
82 if ($name != $main_property_name && $property instanceof PrimitiveInterface) { 83 if ($element_name) { 84 $name = "$element_name-$name"; 85 } 86 $custom_element->setAttribute($name, $property->getValue()); 87 } 52 88 } 53 89 } 54 90 91 /** 92 * {@inheritdoc} 93 */ 94 public function defaultConfiguration() { 95 return parent::defaultConfiguration() + [ 96 'settings' => [ 97 'forceSingleValue' => FALSE, changed this line in version 6 of the diff
added 1 commit
- 60bc6350 - Move flattened functionality into new formatter.
1 <?php 2 3 namespace Drupal\custom_elements\Plugin\CustomElementsFieldFormatter; 4 5 use Drupal\Core\Field\FieldItemListInterface; 6 use Drupal\Core\Form\FormStateInterface; 7 use Drupal\Core\TypedData\PrimitiveInterface; 8 use Drupal\custom_elements\CustomElement; 9 use Drupal\custom_elements\CustomElementsFieldFormatterBase; 10 11 /** 12 * Implementation of the 'flattened' custom element formatter plugin. 13 * 14 * For single-value fields (or when forcing using only a single value), 15 * a field's property values are set in separate attributes (and/or maximum 1 16 * of those values in a slot). Multi-value fields are not supported yet. changed this line in version 8 of the diff
65 /** 66 * {@inheritdoc} 67 */ 68 public function defaultConfiguration() { 69 return parent::defaultConfiguration() + ['force_single_value' => FALSE]; 70 } 71 72 /** 73 * {@inheritdoc} 74 */ 75 public function buildConfigurationForm(array $form, FormStateInterface $form_state) { 76 $form = []; 77 if ($this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() != 1) { 78 $form['force_single_value'] = [ 79 '#type' => 'checkbox', 80 '#title' => $this->t('First field value only'), changed this line in version 8 of the diff
91 $this->configuration['force_single_value'] = $form_state->getValue('force_single_value'); 92 } 93 94 /** 95 * {@inheritdoc} 96 */ 97 public function settingsSummary() { 98 $summary = []; 99 $single_field = $this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() == 1; 100 // This summary line only makes sense for a multi-value field. 101 if (!$single_field && !empty($this->configuration['force_single_value'])) { 102 $summary[] = $this->t('Output only first field value.'); 103 } 104 if (!$single_field && empty($this->configuration['force_single_value'])) { 105 // Temporary message until multivalue is supported. 106 $summary[] = $this->t('Edit to get output; multi-value fields not supported yet.'); changed this line in version 8 of the diff
31 if (empty($this->configuration['settings']['forceSingleValue']) 32 && $this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() != 1) { 33 return; 34 } 35 36 // Process single field value. If the value holds multiple properties: 37 // - Only process 'primitive' properties. (Other ones might be added later.) 38 // - Set the 'main' property in a slot or attribute as configured, with the 39 // configured name - or if that's empty: the original property name. 40 // - Add any other properties as attributes, with 41 // "<configured name>-<own name>". 42 $element_name = $this->getName(); 43 $field_item = $items->first(); 44 $main_property_name = $this->getFieldDefinition()->getFieldStorageDefinition()->getMainPropertyName(); 45 $main_property = $field_item->get($main_property_name); 46 if ($main_property instanceof PrimitiveInterface) { changed this line in version 8 of the diff
added 1 commit
- ba412451 - Simplify formatter, always take only first value; loop through field properties in better way.
14 content: 15 field_link: 16 formatter: flattened 17 name: link 18 is_slot: false 19 weight: 0 20 region: content 21 configuration: 22 is_slot: false 23 settings: 24 trim_length: '80' 25 url_only: '1' 26 url_plain: 0 27 rel: 0 28 target: 0 29 foreceSingleValue: '1' changed this line in version 11 of the diff
63 } 64 65 /** 66 * {@inheritdoc} 67 */ 68 public function defaultConfiguration() { 69 return parent::defaultConfiguration() + ['force_single_value' => FALSE]; 70 } 71 72 /** 73 * {@inheritdoc} 74 */ 75 public function buildConfigurationForm(array $form, FormStateInterface $form_state) { 76 $form = []; 77 if ($this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() != 1) { 78 $form['force_single_value'] = [ reset approvals from @fago by pushing to the branch
15 15 field_link: 16 formatter: raw 16 formatter: flattened 17 17 name: link 18 18 is_slot: false 19 19 weight: 0 20 20 region: content 21 21 configuration: 22 is_slot: false 22 23 settings: 24 trim_length: '80' 25 url_only: '1' 26 url_plain: 0 27 rel: 0 28 target: 0 29 foreceSingleValue: '1' changed this line in version 11 of the diff
18 18 is_slot: false 19 19 weight: 0 20 20 region: content 21 21 configuration: 22 is_slot: false 22 23 settings: 24 trim_length: '80' 25 url_only: '1' 26 url_plain: 0 27 rel: 0 28 target: 0 29 foreceSingleValue: '1' 23 30 forceSingleValue: '1' 24 31 third_party_settings: { } 32 foreceSingleValue: 33 foreceSingleValue: '1' changed this line in version 11 of the diff
reset approvals from @fago by pushing to the branch