Skip to content
Snippets Groups Projects
Commit 416c7c67 authored by Roderik Muit's avatar Roderik Muit Committed by Wolfgang Ziegler
Browse files

Issue #3455435 by roderik: Simplify UI by reverting row names used internally...

Issue #3455435 by roderik: Simplify UI by reverting row names used internally by the form, back to field names
parent 5e337ec4
No related branches found
No related tags found
1 merge request!77Revert row names used internally by the form, back to field names
Pipeline #254208 passed with warnings
...@@ -97,6 +97,40 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -97,6 +97,40 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
); );
} }
/**
* {@inheritdoc}
*/
public function setEntity(EntityInterface $entity) {
// Convert the entity's components to the 'fixed row names' which the form
// can use. Add component property 'name' for the editable names.
$convert_entity_for_form = TRUE;
$converted_components = [];
foreach ($entity->getComponents() as $component_name => $component) {
if (isset($component['name'])) {
// The entity is not the format as loaded from disk (which has no
// 'name'), so apparently it already was converted. This never happens
// in normal operation.
$convert_entity_for_form = FALSE;
break;
}
$component['name'] = $component_name;
$converted_components[$component['field_name']] = $component;
}
if ($convert_entity_for_form) {
$remove_components = array_fill_keys(array_keys($entity->getComponents()), TRUE);
foreach ($converted_components as $row_name => $component) {
$entity->setComponent($row_name, $component);
unset($remove_components[$row_name]);
}
foreach (array_keys($remove_components) as $key) {
$entity->removeComponent($key);
}
}
return parent::setEntity($entity);
}
/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
...@@ -159,7 +193,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -159,7 +193,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
if (!$layout_builder_enabled) { if (!$layout_builder_enabled) {
$form['help'] = [ $form['help'] = [
'#type' => 'markup', '#type' => 'markup',
'#markup' => $this->t("<p><strong>Warning: this user interface is subject to change and contains some bugs. As a result:</strong><ul><li><strong>If you want to rename a 'Key': immediately afterwards, press TAB and then press Save for each individual renamed key.</strong></li><li><strong>When editing anything else, do not rename keys at the same time.</strong></li></ul></p>"), '#markup' => $this->t("<p><strong>Warning: this user interface is subject to change and contains some bugs. As a result:</strong><ul><li><strong>If you want to rename a 'Key': immediately afterwards, press TAB and then press Save for each individual renamed key.</strong></li></ul></p>"),
]; ];
} }
$form = parent::form($form, $form_state); $form = parent::form($form, $form_state);
...@@ -178,13 +212,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -178,13 +212,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
} }
} }
// Remove EntityForm::afterBuild(), which calls copyFormValuesToEntity();
// we cannot update $this->entity before form values are properly validated.
// Any other validation functions must take this into account.
if (!empty($form['#after_build'])) {
$form['#after_build'] = array_diff($form['#after_build'], ['::afterBuild']);
}
return $form; return $form;
} }
...@@ -277,7 +304,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -277,7 +304,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
'#type' => 'textfield', '#type' => 'textfield',
'#title' => $this->t('Attribute / Slot name'), '#title' => $this->t('Attribute / Slot name'),
'#title_display' => 'invisible', '#title_display' => 'invisible',
'#default_value' => $component_name ?: $default_name, '#default_value' => $display_options['name'] ?? $default_name,
'#size' => 20, '#size' => 20,
'#required' => empty($display_options) || $display_options['region'] != 'hidden', '#required' => empty($display_options) || $display_options['region'] != 'hidden',
]; ];
...@@ -323,10 +350,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -323,10 +350,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
'#default_value' => $field_name, '#default_value' => $field_name,
'#attributes' => ['class' => ['field-name']], '#attributes' => ['class' => ['field-name']],
], ],
'existing_name' => [
'#type' => 'hidden',
'#default_value' => $component_name,
],
], ],
'region' => [ 'region' => [
'#type' => 'select', '#type' => 'select',
...@@ -462,65 +485,15 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -462,65 +485,15 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
/** /**
* Gets component name from field name. * Gets component name from field name.
* *
* This is temporary code as long as the UI still has one row per field. It * This is temporary code as long as the UI still has one row per field.
* doesn't check whether the CE display has multiple components with the
* same field name - which isn't supported by the UI yet. There is also no
* check on whether a CE display has 'non-field' components.
*
* WARNING: Re-saving such CE displays through the UI will mess up data!
* *
* @return string * @return string
* The component name. * The component name.
*/
private function getComponentNameFromFieldName(string $field_name, $entity = NULL, $log = TRUE): string {
if (!isset($entity)) {
$entity = $this->entity;
}
$component_name = '';
$components = $entity->getComponents();
foreach ($components as $name => $component) {
if (isset($component['field_name']) && $component['field_name'] == $field_name) {
$component_name = $name;
}
}
if (!$component_name && $log) {
$this->logger('custom_elements')->warning('No component name found for field @field_name. Alpha UI code is unstable.', ['@field_name' => $field_name]);
}
return $component_name;
}
/**
* Form validation callback for the full form.
* *
* @param array $form * @todo remove in #3446485.
* A nested array of form elements comprising the form.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
*/ */
public function validateForm(array &$form, FormStateInterface $form_state) { private function getComponentNameFromFieldName(string $field_name, $entity = NULL, $log = TRUE): string {
parent::validateForm($form, $form_state); return $field_name;
$form_values = $form_state->getValues();
// Validate that no duplicate component name is used.
// @todo there still is a bug with this, as long as the 'name' element
// still has AJAX behavior / triggers requests while renaming individual
// duplicate elements. Fix (remove AJAX behavior) in #3446485.
$component_names_used = [];
foreach ($form['#fields'] as $field_name) {
$name = $form_values['fields'][$field_name]['name'];
if (isset($component_names_used[$name])) {
$form_state->setError($form['fields'][$field_name]['name'], $this->t('Duplicate name %name', ['%name' => $name]));
}
$component_names_used[$name] = TRUE;
}
// If the form isn't being submitted yet: need to set all components in
// the form's entity, so all plugin forms are rebuilt correctly.
if (!$form_state->getErrors() && !empty($form_state->getTriggeringElement()['#ajax'])) {
$this->doCopyFormValuesToEntity($this->entity, $form, $form_state);
}
} }
/** /**
...@@ -539,7 +512,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -539,7 +512,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
$validate_entity = clone $this->entity; $validate_entity = clone $this->entity;
$field_name = $form_state->getTriggeringElement()['#field_name']; $field_name = $form_state->getTriggeringElement()['#field_name'];
$component_name = $this->getComponentNameFromFieldName($field_name, $validate_entity); $component_name = $this->getComponentNameFromFieldName($field_name, $validate_entity);
$this->doCopyFormValuesToEntity($validate_entity, $form, $form_state, $component_name);
// Allow the 'triggering' formatter to validate its configuration. // Allow the 'triggering' formatter to validate its configuration.
$plugin = $validate_entity->getRenderer($component_name); $plugin = $validate_entity->getRenderer($component_name);
...@@ -558,29 +530,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -558,29 +530,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
* {@inheritdoc} * {@inheritdoc}
*/ */
protected function copyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state) { protected function copyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state) {
// See form(): this is only called on submit, with form values validated.
$this->doCopyFormValuesToEntity($entity, $form, $form_state);
}
/**
* Copies form values, or a subset, to entity properties.
*
* This should not change existing entity properties that are not being edited
* by this form.
*
* @param \Drupal\Core\Entity\EntityInterface $entity
* The entity the current form should operate upon.
* @param array $form
* A nested array of form elements comprising the form.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
* @param string $validate_component
* (Optional) Only make sure the basic properties of the specified component
* are updated, in order to be able to validate the full component's config.
* Do not copy the component's full (unvalidated) configuration yet; leave
* other component's values in an undefined state.
*/
protected function doCopyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state, string $validate_component = '') {
assert($entity instanceof EntityCeDisplayInterface); assert($entity instanceof EntityCeDisplayInterface);
$form_values = $form_state->getValues(); $form_values = $form_state->getValues();
if ($this->entity instanceof EntityWithPluginCollectionInterface) { if ($this->entity instanceof EntityWithPluginCollectionInterface) {
...@@ -588,18 +537,12 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -588,18 +537,12 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
$form_values = array_diff_key($form_values, $this->entity->getPluginCollections()); $form_values = array_diff_key($form_values, $this->entity->getPluginCollections());
} }
// 'Rename' components, in a way that components can switch names.
$this->renameEntityComponents($entity, $form, $form_state);
// Collect data for 'regular' fields. // Collect data for 'regular' fields.
// @todo Change this structure in #3446485: $form keys will be component // @todo Change this structure in #3446485: $form keys will be component
// names, not field names. Rename '#fields' (here and elsewhere)? // names, not field names. Rename '#fields' (here and elsewhere)?
foreach ($form['#fields'] as $field_name) { foreach ($form['#fields'] as $field_name) {
$values = $form_values['fields'][$field_name]; $values = $form_values['fields'][$field_name];
$component_name = $values['name']; $component_name = $field_name;
if ($validate_component && $validate_component !== $component_name) {
continue;
}
if ($values['region'] == 'hidden') { if ($values['region'] == 'hidden') {
$entity->removeComponent($component_name); $entity->removeComponent($component_name);
...@@ -608,13 +551,13 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -608,13 +551,13 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
$options = $entity->getComponent($component_name); $options = $entity->getComponent($component_name);
$options['formatter'] = $values['formatter']; $options['formatter'] = $values['formatter'];
$options['field_name'] = $field_name; $options['field_name'] = $field_name;
$options['name'] = $values['name'];
$options['is_slot'] = (bool) $values['is_slot']; $options['is_slot'] = (bool) $values['is_slot'];
$options['weight'] = $values['weight']; $options['weight'] = $values['weight'];
$options['region'] = $values['region']; $options['region'] = $values['region'];
// Update field settings only if the submit handler told us to. Ignore // Update field settings only if the submit handler told us to.
// if we're being called to validate these settings. if ($form_state->get('plugin_settings_update') === $field_name) {
if ($form_state->get('plugin_settings_update') === $field_name && !$validate_component) {
$component_name = $this->getComponentNameFromFieldName($field_name); $component_name = $this->getComponentNameFromFieldName($field_name);
// getRenderer() needs basic properties to be updated. // getRenderer() needs basic properties to be updated.
$entity->setComponent($component_name, $options); $entity->setComponent($component_name, $options);
...@@ -640,60 +583,37 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ...@@ -640,60 +583,37 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase {
} }
/** /**
* Renames CE display components according to form state info. * {@inheritdoc}
*
* This method must be called exactly once on $this->entity, each time a full
* form is submitted (and rebuilt on AJAX submit, or submitted/saved).
* Calling this more than once
* - may result in data corruption if a component is renamed to another
* existing component (if that component is also renamed at the same time;
* otherwise a validation error occurs and this is supposedly not called).
* - will result in warnings (but no data corruption), otherwise. (We can
* therefore assume that data corruption never occurs if no warnings are
* ever logged.)
*
* @param \Drupal\custom_elements\Entity\EntityCeDisplayInterface $entity
* The CE display whose components to rename. The component names must be
* equal to the 'existing_name' form state values, and will get new names
* if applicable.
* @param array $form
* A nested array of form elements comprising the form.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
*
* @todo This setup fails when doing any editing after a rename. It should
* probably be replaced by a setup that keeps the original name in
* $this->entity (with all other changed config) and only does the renaming
* on 'real' submit.
*/ */
protected function renameEntityComponents(EntityCeDisplayInterface $entity, array $form, FormStateInterface $form_state) { public function save(array $form, FormStateInterface $form_state) {
// 'Rename' components, in a way that components can switch names. $components = $this->entity->getComponents();
$new_components = []; // Components are keyed by the form's 'fixed row value'. Key them by
$form_values = $form_state->getValues(); // component name as defined the schema.
// @todo Change this structure in #3446485: $form keys will be component $remove_components = array_fill_keys(array_keys($components), TRUE);
// names, not field names. Rename '#fields' (here and elsewhere)? $set_components = [];
foreach ($form['#fields'] as $field_name) { foreach ($components as $form_row_name => $component) {
$values = $form_values['fields'][$field_name]; $component_name = $component['name'];
$component_name = $values['name']; if (isset($set_components[$component_name])) {
if ($values['region'] != 'hidden') { // Duplicate name; the first one is more likely to be the one we want.
if (isset($values['parent_wrapper']['existing_name'])) { // Do not cancel; just don't save.
$existing_name = $values['parent_wrapper']['existing_name']; // @todo figure out duplicate name detection after #3446485.
if ($existing_name && $component_name !== $existing_name) { $this->messenger()->addWarning(
// Old component can be empty (and populated later) if it used to $this->t('Several components have the key %name. The component for row %row_name was not saved.',
// be hidden. ['%name' => $component_name, '%row_name' => $form_row_name]
$new_components[$component_name] = $entity->getComponent($existing_name) ?? []; ));
$entity->removeComponent($existing_name); continue;
}
}
else {
// This should never happen. Are we being called twice?
$this->logger('custom_elements')->warning('Field row for @field_name somehow has no existing-name. (This suggests it is being renamed more than once.)', ['@field_name' => $field_name]);
}
} }
unset($component['name']);
$this->entity->setComponent($component_name, $component);
unset($remove_components[$component_name]);
$set_components[$component_name] = TRUE;
} }
foreach ($new_components as $component_name => $component) { foreach (array_keys($remove_components) as $key) {
$entity->setComponent($component_name, $component); $this->entity->removeComponent($key);
} }
return parent::save($form, $form_state);
} }
/** /**
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment