Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds new Action Option :self #546

Merged
merged 9 commits into from Jul 17, 2022

Conversation

radiantshaw
Copy link
Contributor

This PR adds two new Action Options, namely :self and :!self, as discussed in #530. For more context, please refer to the original issue.

Feel free to provide any suggestions and I'll make sure to accommodate them.

@radiantshaw
Copy link
Contributor Author

Seems like some of the older commits are also showing up here. I'll rebase with the latest main branch and re-push when I get the time. 👍

@radiantshaw radiantshaw force-pushed the self-and-not-self-action-options branch from d52e9ee to 615e8ab Compare May 29, 2022 17:40
@radiantshaw
Copy link
Contributor Author

Done. I've rebased the branch with the latest main. 🚀

@rik
Copy link
Contributor

rik commented May 29, 2022

I quite liked the clear distinction proposed in #530 (comment):

  • describe event options with trailing :option tokens
  • filter events with .variation token (probably only one in this case)

I think it will also read better: <button data-action="click.self->hello#greet"> vs <button data-action="click->hello#greet:self">


Is there a use case for !self ? I can't think of one.

@radiantshaw
Copy link
Contributor Author

@rik Sorry for replying late on this. I've been trying to also think of some use case for !self but can't. I'll think of it for some more time and get back on this.

@dhh
Copy link
Member

dhh commented Jul 15, 2022

Let's just go with self alone?

@radiantshaw
Copy link
Contributor Author

@dhh Yes. I didn't find any use case for the :!self option. It also makes sense for it to be .self instead of :self as @rik mentioned. So I'll try to refactor the code.

I was a bit busy so I didn't get much time to work on this. But I do plan on working on it in the upcoming weekends. ✌️

@marcoroth
Copy link
Member

I think we should keep it as :self so it's in line with all the other event modifiers

@radiantshaw
Copy link
Contributor Author

@marcoroth I'm okay with either. But I also agree with @rik and feel that self falls into the "filter" category instead of "event options" and it would be better if we keep it as .self instead of :self. But I'm open to feedback on why we should have it as :self as well.

@dhh
Copy link
Member

dhh commented Jul 17, 2022

Think we want to stick with :self to get chaining. You could want both self + prevent etc.

@rik
Copy link
Contributor

rik commented Jul 17, 2022

Think we want to stick with :self to get chaining. You could want both self + prevent etc.

I don't understand this point. The syntax proposed by @adrienpoly allows for self + prevent. You left a rocket emoji reaction to this proposal. I'm copying the examples here:

# mixing event variant and event options
<div data-action="click.left->controller#vote:once:prevent">

# with global listener
<div data-action="keydown.left@window->tabs#prevTab">

@dhh
Copy link
Member

dhh commented Jul 17, 2022

@rik Sorry, I meant mixing things like left and self. IMO, self feels more like once/prevent than it does "left". Don't like the feel of "click.left.self->" either, even if that could be possible.

@dhh dhh merged commit 910f2fd into hotwired:main Jul 17, 2022
@seanpdoyle
Copy link
Contributor

seanpdoyle commented Jul 18, 2022

I'm sorry to chime in so late, but I'm at a loss understanding :self's utility. I read through the original issue, but couldn't find an example use case. Could you share one here?

Are :self's semantics similar to checking an event's currentTarget as it bubbles up the DOM? Was :currentTarget ever considered as a modifier name?

@dhh dhh changed the title Adds new Action Options, namely :self and :!self Adds new Action Option :self Jul 18, 2022
@dhh
Copy link
Member

dhh commented Jul 18, 2022

@seanpdoyle Good explanation from the same feature in Vue: https://stackoverflow.com/questions/42763940/vue-js-what-is-the-self-modifier

seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[turbo-events]: https://turbo.hotwired.dev/reference/events
seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events
seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events
seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events
seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events
seanpdoyle added a commit to seanpdoyle/stimulus that referenced this pull request Jul 28, 2022
As a follow-up to [hotwired#535][] and
[hotwired#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[hotwired#535]: hotwired#535
[hotwired#546]: hotwired#546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events
dhh pushed a commit that referenced this pull request Jul 28, 2022
* Support custom Action Options

As a follow-up to [#535][] and
[#546][], add support for declaring custom action
modifiers in the same style as `:prevent`, `:stop`, and `:self`.

Take, for example, the [toggle][] event. It's dispatched whenever a
`<details>` element toggles either open or closed. If an application
were able to declare a custom `open` modifier, it could choose to route
`toggle` events denoted with `:open` _only_ when the `<details open>`.
Inversely, they could choose to route `toggle` events denoted with
`:!open` _only_ when the `<details>` does not have `[open]`.

Similarly, the same kind of customization could apply to custom events.
For example, the [turbo:submit-end][turbo-events] fires after a `<form>`
element submits, but does not distinguish between success or failure. A
`:success` modifier could skip events with an unsuccessful HTTP response
code.

[#535]: #535
[#546]: #546
[toggle]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDetailsElement/toggle_event
[turbo-events]: https://turbo.hotwired.dev/reference/events

* attempt to pass on Safari@14

failing test: https://github.com/hotwired/stimulus/runs/7566084180?check_suite_focus=true#step:6:138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants