🇯🇵Japan @tom konda

I rebased feature branch and added a test for menu depth option.
But, need to fix SitemapUpdateSettingsTest::testUpdateHookN.

tom konda → made their first commit to this issue’s fork.

Issue #3176383 is removed.
IE11 had been dropped support since Drupal 10. No need to add polyfill.

In my opinion, other instances cannot be replaced.

I added type checking for a JavaScript theme function.
Please review.

tom konda → created an issue.

tom konda → created an issue.

@avpaderno Oh, sorry. I misunderstand that MR!6 is valid branch.

I fixed MR !6 and all validate stage was all green.
Please review.

tom konda → created an issue.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

I think this issue is already solved in 🐛 Drupal coding standards Needs work .

I fix all of ESLint errors and all of validate stages are green.
Please review.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

Add testing code for template literal.
Please review.

I reran Functional JavaScript tests and all tests are passed.
It is probably random failure.
Please review.

Ran Functional JavaScript tests several times.
But there is an error on Drupal\Tests\ckeditor5\FunctionalJavascript\ImageUrlProviderTest.
Need to resolve this error.

tom konda → created an issue.

tom konda → created an issue.

Fix ESLint probrem.
Please review.

tom konda → created an issue.

I resolved merge conflict and stylelint is green.
Please review.

tom konda → made their first commit to this issue’s fork.

I rebased MR and fixed all of ESLint errors for latest release.
ESLint is green now. Please review.

tom konda → made their first commit to this issue’s fork.

I rebased the issue branch and fixed some of validate stage errors.

tom konda → made their first commit to this issue’s fork.

I rebased the issue branch and merge error is resolved.
Please review.

I rebased the issue branch and fixed no-var errors.
There are following ESLint probrems left.

• no-undef
• no-new-func
• no-restricted-syntax

Fix ESLint error and rebuild CKEditor5 plugin JavaScript file.

I fixed phpcs errors and phpcs tests are all green.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

I fixed following ESLint errors. But still there is ESLint errors, so need to work.

• ESLint fixable errors
• Camelcase errors
• Eqeqeq errors
• A no-restricted-globals error
• Errors after line 278 except no-undef error

tom konda → made their first commit to this issue’s fork.

Fix pipeline ESLint errors.
Please review.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

tom konda → created an issue.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

tom konda → created an issue.

I investigated a FunctionalJavaScriptTests behavior with the drupalci/chromedriver:dev container and found set wrong value to event.key and event.code on my local environment.
Behat\Mink\Element\NodeElement::keyDown() only calls driver's keyDown() method, so may have a problem on WebDriver or other.

Investigation method

1. Change /core/misc/tabledrag.js to record some KeyboardEvent value to drupalSetting on a keydown event. (Code 1)
2. Change /core/tests/Drupal/FunctionalJavascriptTests/TableDrag/TableDragTest.php to output drupalSettings.keyTest value. (Code 2)
3. Test /core/tests/Drupal/FunctionalJavascriptTests/TableDrag/TableDragTest.php

Code 1

    // On blur, fire the same function as a touchend/mouseup. This is used to
    // update values after a row has been moved through the keyboard support.
    $handle.on('blur', (event) => {
      if (self.rowObject && self.safeBlur) {
        self.dropRow(event, self);
      }
    });

    // Add arrow-key support to the handle.
    $handle.on('keydown', (event) => {
      // I added below code for investigation.
      const {charCode, keyCode, code, key} = event;
      drupalSettings.keyTest = {
        charCode, keyCode, code, key
      };
      // I added above code for investigation.

Code 2

  protected function moveRowWithKeyboard(NodeElement $row, $arrow, $repeat = 1): void {
    $keys = [
      'left' => 37,
      'right' => 39,
      'up' => 38,
      'down' => 40,
    ];
    if (!isset($keys[$arrow])) {
      throw new \InvalidArgumentException('The arrow parameter must be one of "left", "right", "up" or "down".');
    }

    $key = $keys[$arrow];

    $handle = $row->find('css', 'a.tabledrag-handle');
    $handle->focus();

    for ($i = 0; $i < $repeat; $i++) {
      $this->markRowHandleForDragging($handle);
      $handle->keyDown($key);
      // I added below code for investigation.
      print_r($this->getDrupalSettings()['keyTest']);
      // I added above code for investigation.
      $handle->keyUp($key);
      $this->waitUntilDraggingCompleted($handle);
    }

    $handle->blur();
  }

Result

PHPUnit 10.5.38 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.13
Configuration: /opt/drupal/web/core/phpunit.xml.dist

..Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
FArray
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 37
    [code] => 
    [key] => %
    [keyCode] => 37
)
FArray
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 37
    [code] => 
    [key] => %
    [keyCode] => 37
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
FArray
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
F                                                              6 / 6 (100%)Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)

/core/modules/locale/js/locale.admin.js uses event.code instead of event.key because event.key when space key is pressed, returns ' ' (a white space character) which make it little difficult code reading.

tom konda → created an issue.

> let's create an issue for the unicorn plugin
I created an issue for the unicorn plugin.
📌 Consider to use some of plugin:unicorn/recommended ESLint rules for Drupal core Active

tom konda → created an issue.

tom konda → created an issue.

I found minimist was called in Nightwatch testing codes.
Need to fix these codes.

tom konda → created an issue.

Fix for 8.x-1.x branch.
Please review.

tom konda → created an issue.

Yes, but need to add "eslint-plugin-unicorn" as package.json devDependencies and fix ESLint settings to use "plugin:unicorn/recommended". Especially "plugin:unicorn/recommended" rules effects many of things so I think need to discuss that which rules to be enabled for the Drupal core project.

tom konda → created an issue.

tom konda → created an issue.