Kanagawa, Japan
Account created on 17 September 2016, over 8 years ago
#

Merge Requests

More

Recent comments

🇯🇵Japan tom konda Kanagawa, Japan

Add testing code for template literal.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

Fix ESLint probrem.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

  • no-undef
  • no-new-func
  • no-restricted-syntax
🇯🇵Japan tom konda Kanagawa, Japan

Fix ESLint error and rebuild CKEditor5 plugin JavaScript file.

🇯🇵Japan tom konda Kanagawa, Japan

I fixed phpcs errors and phpcs tests are all green.

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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
🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

Fix pipeline ESLint errors.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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
)
🇯🇵Japan tom konda Kanagawa, Japan

/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.

🇯🇵Japan tom konda Kanagawa, Japan

> 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

🇯🇵Japan tom konda Kanagawa, Japan

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

🇯🇵Japan tom konda Kanagawa, Japan

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.

🇯🇵Japan tom konda Kanagawa, Japan

Fix remaining Array.prototype.indexOf() on the 10.4.x branch.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

10.4.x branch has tracker module.
In this module still have Array.prototype.indexOf() so need to fix it.

🇯🇵Japan tom konda Kanagawa, Japan

Reran the pipeline and all tests are passed.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

I changed to simple else clause in my local environment, it seems to work correctly.

🇯🇵Japan tom konda Kanagawa, Japan

Reroll a patch for 11.x branch and create the MR 7672 as draft.
Some of functional JavaScript tests are failed, so need to resolve.

🇯🇵Japan tom konda Kanagawa, Japan

I think this MR looks good.
Please review.

🇯🇵Japan tom konda Kanagawa, Japan

Merge test file changing at commit 05e4d1a8 to the MR and all tests are passed.
Please review.

Production build 0.71.5 2024