Allow action params for global events

[rik]

Ping @adrienpoly since you implemented the original version and you might have some insight why you used the event target rather than the element.

[seanpdoyle]

Thank you for opening this PR!

I tried this locally, and I was able to pass the tests you've written with a smaller set of changes:

diff --git a/src/core/action.ts b/src/core/action.ts
index 5c69dc9..6a69d1f 100644
--- a/src/core/action.ts
+++ b/src/core/action.ts
@@ -34,7 +34,7 @@ export class Action {
     if (this.eventTarget instanceof Element) {
       return this.getParamsFromEventTargetAttributes(this.eventTarget)
     } else {
-      return {}
+      return this.getParamsFromEventTargetAttributes(this.element)
     }
   }
 
diff --git a/src/tests/modules/core/action_params_tests.ts b/src/tests/modules/core/action_params_tests.ts
index afdbab4..982c03c 100644
--- a/src/tests/modules/core/action_params_tests.ts
+++ b/src/tests/modules/core/action_params_tests.ts
@@ -16,6 +16,7 @@ export default class ActionParamsTests extends LogControllerTestCase {
         <div id="nested"></div>
       </button>
     </div>
+    <div id="outside"></div>
   `
   expectedParamsForC = {
     id: 123,
@@ -38,6 +39,16 @@ export default class ActionParamsTests extends LogControllerTestCase {
     )
   }
 
+  async "test global event return element params where the action is defined"() {
+    this.actionValue = "keydown@window->c#log"
+    await this.nextFrame
+    await this.triggerEvent("#outside", "keydown")
+
+    this.assertActions(
+      { identifier: "c", params: this.expectedParamsForC },
+    )
+  }
+
   async "test passing params to namespaced controller"() {
     this.actionValue = "click->c#log click->d#log2"
     await this.nextFrame

Is there something I'm misunderstanding that makes the rest of the implementation change necessary?

[rik]

I was trying to cleanup after making the code work.

The smallest change I initially made was:

  get params(): object {
    return this.getParamsFromEventTargetAttributes(this.element)
  }

But then getParamsFromEventTargetAttributes and its argument are misnamed since it's not working with event targets anymore. Instead of renaming it, I moved it inside the params getter. That makes for a bigger fix but the resulting code does not have confusing identifiers.

For the more subjective changes, they are not needed at all. That would be moving from foreach to for/of and from Object.assign to params[camelize(key)]= typecast(value).

[adrienpoly]

Thanks @rik, it corrects a significant bug for action params and global events. I think I was confused by this.element in the action context vs this.element in the typical Stimulus context.

This makes the solution even simpler

[rik]

@dhh Is there a nightly build somewhere to start using this fix? Thanks!

[pokonski]

Just came across this issue, too - a stable release with this fix would be great :)