From ded815fbe0a6f84dc9826a5027218f5df2a48f42 Mon Sep 17 00:00:00 2001 From: Lauri Eskola Date: Thu, 17 Aug 2023 16:45:36 +0300 Subject: [PATCH] Issue #3370113 by Utkarsh_33, lauriii, omkar.podey, tedbow, bnjmnm, ckrina, smustgrave, longwave, hooroomoo, srishtiiee, yoroy: Make it easier to enter multiple values to fields allowing unlimited values --- core/core.libraries.yml | 8 + core/misc/field-list-keyboard-navigation.js | 69 ++++++++ core/misc/machine-name.js | 16 +- .../Plugin/Field/FieldType/ListItemBase.php | 25 ++- .../OptionsFieldUITest.php | 148 +++++++++++++++++- 5 files changed, 256 insertions(+), 10 deletions(-) create mode 100644 core/misc/field-list-keyboard-navigation.js diff --git a/core/core.libraries.yml b/core/core.libraries.yml index 52245fc928b..9f17f189f50 100644 --- a/core/core.libraries.yml +++ b/core/core.libraries.yml @@ -953,3 +953,11 @@ js-cookie: js: assets/vendor/js-cookie/js.cookie.min.js: {} deprecated: The %library_id% asset library is deprecated in Drupal 10.1.0 and will be removed in Drupal 11.0.0. There is no replacement. See https://www.drupal.org/node/3322720 + +drupal.fieldListKeyboardNavigation: + version: VERSION + js: + misc/field-list-keyboard-navigation.js: {} + dependencies: + - core/drupal + - core/tabbable diff --git a/core/misc/field-list-keyboard-navigation.js b/core/misc/field-list-keyboard-navigation.js new file mode 100644 index 00000000000..5e932e4c956 --- /dev/null +++ b/core/misc/field-list-keyboard-navigation.js @@ -0,0 +1,69 @@ +/** + * @file + * Attaches behaviors for Drupal's field list keyboard navigation. + */ +(function (Drupal, { isFocusable }) { + /** + * Attaches the focus shifting functionality. + * + * @type {Drupal~behavior} + * + * @prop {Drupal~behaviorAttach} attach + * Attaches behaviors. + */ + Drupal.behaviors.fieldListKeyboardNavigation = { + attach() { + once( + 'keyboardNavigation', + 'input[type="text"], input[type="number"]', + document.querySelector('[data-field-list-table]'), + ).forEach((element) => + element.addEventListener('keypress', (event) => { + if (event.key !== 'Enter') { + return; + } + event.preventDefault(); + const currentElement = event.target; + + // Function to find the next focusable element. + const findNextFocusableElement = (element) => { + const currentRow = element.closest('tr'); + const inputElements = currentRow.querySelectorAll( + 'input[type="text"], input[type="number"]', + ); + const afterIndex = [...inputElements].indexOf(element) + 1; + + // eslint-disable-next-line no-restricted-syntax + for (const inputElement of [...inputElements].slice(afterIndex)) { + if (isFocusable(inputElement)) { + return inputElement; + } + } + const nextRow = currentRow.nextElementSibling; + if (nextRow) { + return findNextFocusableElement(nextRow); + } + return null; + }; + + const nextFocusableElement = findNextFocusableElement(currentElement); + + // If a focusable element is found, move focus there. + if (nextFocusableElement) { + nextFocusableElement.focus(); + // Move cursor to the end of the input. + const value = nextFocusableElement.value; + nextFocusableElement.value = ''; + nextFocusableElement.value = value; + return; + } + // If no focusable element is found, add another item to the list. + event.target + .closest('[data-field-list-table]') + .parentNode.querySelector('[data-field-list-button]') + .dispatchEvent(new Event('mousedown')); + }), + ); + }, + }; +})(Drupal, window.tabbable); diff --git a/core/misc/machine-name.js b/core/misc/machine-name.js index 7cf8bdd9f15..b4e06d595c1 100644 --- a/core/misc/machine-name.js +++ b/core/misc/machine-name.js @@ -207,7 +207,21 @@ ''), - ).on('click', eventData, clickEditHandler); + ) + .on('click', eventData, clickEditHandler) + .on('keyup', (e) => { + // Avoid propagating a keyup event from the machine name input. + if (e.key === 'Enter' || eventData.code === 'Space') { + e.preventDefault(); + e.stopImmediatePropagation(); + e.target.click(); + } + }) + .on('keydown', (e) => { + if (e.key === 'Enter' || eventData.code === 'Space') { + e.preventDefault(); + } + }); $suffix.append($link); // Preview the machine name in realtime when the human-readable name diff --git a/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php b/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php index e22a6e18ba7..9e5f704a2f7 100644 --- a/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php +++ b/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php @@ -4,6 +4,9 @@ namespace Drupal\options\Plugin\Field\FieldType; use Drupal\Component\Utility\Html; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Ajax\AjaxResponse; +use Drupal\Core\Ajax\FocusFirstCommand; +use Drupal\Core\Ajax\InsertCommand; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemBase; use Drupal\Core\Form\FormStateInterface; @@ -118,6 +121,7 @@ abstract class ListItemBase extends FieldItemBase implements OptionsProviderInte ], '#attributes' => [ 'id' => 'allowed-values-order', + 'data-field-list-table' => TRUE, ], '#tabledrag' => [ [ @@ -126,6 +130,9 @@ abstract class ListItemBase extends FieldItemBase implements OptionsProviderInte 'group' => 'weight', ], ], + '#attached' => [ + 'library' => ['core/drupal.fieldListKeyboardNavigation'], + ], ]; $max = $form_state->get('items_count'); @@ -196,12 +203,14 @@ abstract class ListItemBase extends FieldItemBase implements OptionsProviderInte } } $element['allowed_values']['table']['#max_delta'] = $max; - $element['allowed_values']['add_more_allowed_values'] = [ '#type' => 'submit', '#name' => 'add_more_allowed_values', '#value' => $this->t('Add another item'), - '#attributes' => ['class' => ['field-add-more-submit']], + '#attributes' => [ + 'class' => ['field-add-more-submit'], + 'data-field-list-button' => TRUE, + ], // Allow users to add another row without requiring existing rows to have // values. '#limit_validation_errors' => [], @@ -210,6 +219,10 @@ abstract class ListItemBase extends FieldItemBase implements OptionsProviderInte 'callback' => [static::class, 'addMoreAjax'], 'wrapper' => $wrapper_id, 'effect' => 'fade', + 'progress' => [ + 'type' => 'throbber', + 'message' => $this->t('Adding a new item...'), + ], ], ]; @@ -246,10 +259,14 @@ abstract class ListItemBase extends FieldItemBase implements OptionsProviderInte // Go one level up in the form. $element = NestedArray::getValue($form, array_slice($button['#array_parents'], 0, -1)); $delta = $element['table']['#max_delta']; - $element['table'][$delta]['item']['#prefix'] = '
' . ($element['table'][$delta]['item']['#prefix'] ?? ''); + $element['table'][$delta]['item']['#prefix'] = '
' . ($element['table'][$delta]['item']['#prefix'] ?? ''); $element['table'][$delta]['item']['#suffix'] = ($element['table'][$delta]['item']['#suffix'] ?? '') . '
'; - return $element; + $response = new AjaxResponse(); + $response->addCommand(new InsertCommand(NULL, $element)); + $response->addCommand(new FocusFirstCommand('[data-drupal-selector="field-list-add-more-focus-target"]')); + + return $response; } /** diff --git a/core/modules/options/tests/src/FunctionalJavascript/OptionsFieldUITest.php b/core/modules/options/tests/src/FunctionalJavascript/OptionsFieldUITest.php index 9119628ac36..56d39c0b4dd 100644 --- a/core/modules/options/tests/src/FunctionalJavascript/OptionsFieldUITest.php +++ b/core/modules/options/tests/src/FunctionalJavascript/OptionsFieldUITest.php @@ -79,7 +79,8 @@ class OptionsFieldUITest extends WebDriverTestBase { * * @dataProvider providerTestOptionsAllowedValues */ - public function testOptionsAllowedValues($option_type, $options, $is_string_option) { + public function testOptionsAllowedValues($option_type, $options, $is_string_option, string $add_row_method) { + $assert = $this->assertSession(); $this->fieldName = 'field_options_text'; $this->createOptionsField($option_type); $page = $this->getSession()->getPage(); @@ -87,15 +88,79 @@ class OptionsFieldUITest extends WebDriverTestBase { $this->drupalGet($this->adminPath); $i = 0; + $expected_rows = 1; + $this->assertAllowValuesRowCount(1); foreach ($options as $option_key => $option_label) { - $page->fillField("settings[allowed_values][table][$i][item][label]", $option_label); + $enter_element_name = $label_element_name = "settings[allowed_values][table][$i][item][label]"; + $page->fillField($label_element_name, $option_label); + $key_element_name = "settings[allowed_values][table][$i][item][key]"; + // Add keys if not string option list. if (!$is_string_option) { - $page->fillField("settings[allowed_values][table][$i][item][key]", $option_key); + $this->pressEnterOnElement("[name=\"$label_element_name\"]"); + // Assert that pressing enter on label field does not create the new + // row if the key field is visible. + $this->assertAllowValuesRowCount($expected_rows); + $enter_element_name = $key_element_name; + $this->assertHasFocusByAttribute('name', $key_element_name); + $page->fillField($key_element_name, $option_key); } - $page->pressButton('Add another item'); + else { + $this->assertFalse($assert->fieldExists($key_element_name)->isVisible()); + } + switch ($add_row_method) { + case 'Press button': + $page->pressButton('Add another item'); + break; + + case 'Enter button': + $button = $assert->buttonExists('Add another item'); + $this->pressEnterOnElement('[data-drupal-selector="' . $button->getAttribute('data-drupal-selector') . '"]'); + break; + + case 'Enter element': + // If testing using the "enter" key while focused on element there a + // few different scenarios to test. + switch ($i) { + case 0: + // For string options the machine name input can be exposed which + // will mean the label input will no longer create the next row. + if ($is_string_option) { + $this->exposeOptionMachineName($expected_rows); + $this->pressEnterOnElement("[name=\"$enter_element_name\"]"); + $this->assertHasFocusByAttribute('name', $key_element_name); + // Ensure that pressing enter while focused on the label input + // did not create a new row if the machine name field is + // visible. + $this->assertAllowValuesRowCount($expected_rows); + $enter_element_name = $key_element_name; + } + break; + } + $this->pressEnterOnElement("[name=\"$enter_element_name\"]"); + break; + + default: + throw new \UnexpectedValueException("Unknown method $add_row_method"); + } + $i++; + $expected_rows++; $this->assertSession()->waitForElementVisible('css', "[name='settings[allowed_values][table][$i][item][label]']"); + $this->assertHasFocusByAttribute('name', "settings[allowed_values][table][$i][item][label]"); + $this->assertAllowValuesRowCount($expected_rows); + + if ($is_string_option) { + // Expose the key input for string options for the previous row to test + // shifting focus from the label to key inputs on the previous row by + // pressing enter. + $this->exposeOptionMachineName($expected_rows - 1); + } + // Test that pressing enter on the label input on previous row will shift + // focus to key input of that row. + $this->pressEnterOnElement("[name=\"$label_element_name\"]"); + $this->assertHasFocusByAttribute('name', $key_element_name); + $this->assertAllowValuesRowCount($expected_rows); } $page->pressButton('Save field settings'); @@ -200,6 +265,21 @@ class OptionsFieldUITest extends WebDriverTestBase { $this->adminPath = 'admin/structure/types/manage/' . $this->type . '/fields/node.' . $this->type . '.' . $this->fieldName . '/storage'; } + /** + * Presses "Enter" on the specified element. + * + * @param string $selector + * Current element having focus. + */ + private function pressEnterOnElement(string $selector): void { + $javascript = <<getSession()->executeScript($javascript); + } + /** * Data provider for testOptionsAllowedValues(). * @@ -208,9 +288,11 @@ class OptionsFieldUITest extends WebDriverTestBase { * - Option type. * - Array of option type values. * - Whether option type is string type or not. + * - The method which should be used to add another row to the table. The + * possible values are 'Press button', 'Enter button' or 'Enter element'. */ public function providerTestOptionsAllowedValues() { - return [ + $type_cases = [ 'List integer' => [ 'list_integer', [1 => 'First', 2 => 'Second', 3 => 'Third'], @@ -227,6 +309,62 @@ class OptionsFieldUITest extends WebDriverTestBase { TRUE, ], ]; + // Test adding options for each option field type using several possible + // methods that could be used for navigating the options list: + // - Press button: add a new item by pressing the 'Add another item' + // button using mouse. + // - Enter button: add a new item by pressing the 'Add another item' + // button using enter key on the keyboard. + // - Enter element: add a new item by pressing enter on the last text + // field inside the table. + $test_cases = []; + foreach ($type_cases as $key => $type_case) { + foreach (['Press button', 'Enter button', 'Enter element'] as $add_more_method) { + $test_cases["$key: $add_more_method"] = array_merge($type_case, [$add_more_method]); + } + } + return $test_cases; + } + + /** + * Assert the count of the allowed values rows. + * + * @param int $expected_count + * The expected row count. + */ + private function assertAllowValuesRowCount(int $expected_count): void { + $this->assertCount( + $expected_count, + $this->getSession()->getPage()->findAll('css', '#allowed-values-order tr.draggable') + ); + } + + /** + * Asserts an element specified by an attribute value has focus. + * + * @param string $name + * The attribute name. + * @param string $value + * The attribute value. + * + * @todo Replace with assertHasFocus() in https://drupal.org/i/3041768. + */ + private function assertHasFocusByAttribute(string $name, string $value): void { + $active_element = $this->getSession()->evaluateScript('document.activeElement'); + $this->assertSame($value, $active_element->getAttribute($name)); + } + + /** + * Exposes the machine name input for a row. + * + * @param int $row + * The row number. + */ + private function exposeOptionMachineName(int $row): void { + $index = $row - 1; + $rows = $this->getSession()->getPage()->findAll('css', '#allowed-values-order tr.draggable'); + $this->assertSession()->buttonExists('Edit', $rows[$index])->click(); + $this->assertSession()->waitForElementVisible('css', "[name='settings[allowed_values][table][$index][item][key]']"); } }