alpinejs / alpine

A rugged, minimal framework for composing JavaScript behavior in your markup.
https://alpinejs.dev
MIT License
27.92k stars 1.22k forks source link

Fix x-on with both self and once #4152

Closed bb closed 4 months ago

bb commented 4 months ago

I want to use the once and self modifiers together, like @something.self.once, but the handler never runs if the first event is not from self. I think the reason is that once is evaluated before self.

This PR fixes the order of evaluating the modifiers so the two work together.

I think the failing test is unrelated.

bb commented 4 months ago

I'm not sure if I understand your request correctly. Keydown and keydown are already lexically last, i.e. evaluated first.

But maybe we want to move once up a bit, so e.g. clicking outside (once) will actually work after a click inside.

bb commented 4 months ago

I added a test for .away.once which was failing and fixed it by moving once lexically higher, i.e. evaluated later than the inside/outside check.

Tests for keyup.once already existed and passed before this PR.

Tim-Wils commented 4 months ago

Shouldn't 'once' be part of the options passed to the addEventListener though? That way the eventListener destroy itself after and the removeEventListener part could be skipped/removed.

Once option docs: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#once

bb commented 4 months ago

That wouldn't work with the filtering modifiers, @Tim-Wils. Let's take keydown as an example: with plain event listeners, you can only listen for keydown, but not for a specific key. Try this:

window.addEventListener('keydown', function(event) {
  if (event.key === 'b') {
    console.log('pressed b once');
  }
}, { once: true });

Press first "a". Then press "b". Nothing will be logged on the console, because after pressing a, the listener would be gone.

Tim-Wils commented 4 months ago

@bb Seems fair, you're right :) Thanks for the example!

ekwoka commented 4 months ago

Yes last or first. Once should do it's thing only once all the others have passed, no?

If once is after self, it should also be after the keyup modifiers. Since they should both only do the once if they pass.

bb commented 4 months ago

Exactly. That's how I did it from the beginning and how it still is.

Order of processing in as of the current state of this PR:

  1. keys
  2. self
  3. away/outside
  4. once

So, what's your request?

calebporzio commented 4 months ago

Looks great and makes perfect sense. Thanks @bb!

bb commented 4 months ago

Thanks for merging and for AlpineJS in general 😊