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

Merge Requests

More

Recent comments

🇯🇵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.

🇯🇵Japan tom konda Kanagawa, Japan

MR has conflict because of testing file changing at commit 05e4d1a8. Need to resolve this conflict.

🇯🇵Japan tom konda Kanagawa, Japan

@sakthi_dev After commit id 135e3ba1 original script size is changed, so need to recalculate script size.

🇯🇵Japan tom konda Kanagawa, Japan

@nod_ No, I think no rule is found in ESLint plugins for vanilla JavaScript at this point.
"typescript-eslint" plugin has ESLint rule but this rule is for TypeScript.

🇯🇵Japan tom konda Kanagawa, Japan

Some of PHPUnit Functional Javascript tests and Nightwatch tests are failed.
Need to resolve failed tests.

🇯🇵Japan tom konda Kanagawa, Japan

I think using URLSearchParams() instead of RegExp(), it makes code more simple than patch #49.
I committed for the 2508796-query-string-for-11.x branch.

🇯🇵Japan tom konda Kanagawa, Japan

I changed existing 'method' => 'replace' to 'method' => 'replaceWith' in test modules.
But core/lib/Drupal/Core/Render/Element/RenderElement.php has conflict, so need to resolve.

🇯🇵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

So meanwhile the polyfill is only needed for Opera Mini

According to #3202818: Remove untrue declaration of Opera Mini (in reverse proxy mode) support from Drupal 9.3 , Opera Mini in 'High Data Savings' mode should work as well as Opera Mobile and "Can I Use" say classList feature is supported on Opera Mobile.
Opera Mini in 'Extreme Data Savings' mode had been dropped support since Drupal 9.3.

I think no need to take further action for this issue.

Production build 0.71.5 2024