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

[invokers] Should events trigger when invoking? #1033

Open
brechtDR opened this issue Apr 9, 2024 · 5 comments
Open

[invokers] Should events trigger when invoking? #1033

brechtDR opened this issue Apr 9, 2024 · 5 comments
Labels

Comments

@brechtDR
Copy link
Collaborator

brechtDR commented Apr 9, 2024

When creating a demo with invoker buttons, I was wondering if a change event on an input should be triggered when controlling it, or just events in general if we'd like to take this further. At the moment, this doesn't happen.

The example I'm referring to is using invokers to trigger an <input type="number" />

The example I was making: https://codepen.io/utilitybend/pen/poBLLWy/81246d701205fadefbbcc1fbe2dd72cf

Code example in short:

  <button invoketarget="num" invokeaction="stepDown">
        Decrease number
  </button>
  <input type="number" id="num">
  <button invoketarget="num" invokeaction="stepUp">
        Increase number
  </button>

Should the following trigger:

const numberInput = document.querySelector('input[type="number"]');

numberInput.addEventListener("change", () => {
    // Currently, i don't trigger, should I?
});

Would love to hear thoughts on this. It would made my demo easier, but that's currently the only argument I have for it. Wanted to open this to the group

@lukewarlow lukewarlow changed the title [invokers] Should JS methods trigger when invoking? [invokers] Should events trigger when invoking? Apr 9, 2024
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 11, 2024

That definitely seems like a bug to me. If an input changes, a change event should be fired. Perhaps more nebulous would be the input event. I can see a case for not firing that one? Not sure.

@lukewarlow
Copy link
Collaborator

lukewarlow commented Apr 11, 2024

That definitely seems like a bug to me. If an input changes, a change event should be fired. Perhaps more nebulous would be the input event. I can see a case for not firing that one? Not sure.

So the change event doesn't fire for the JS API for stepUp or stepDown across all 3 engines. I agree that input shouldn't be fired as that's quite clearly about user input.

Here's the demo to show change not firing: https://jsfiddle.net/9nrp7b2u/

@brechtDR
Copy link
Collaborator Author

Also want to note that when you fire the "invoke" event, the input's value will be updated after the actual event (and thus return the previous value). this is by design, but it does make it so that triggering change event would be extra handy. Drawing a bit of a blank to explain this with words, so here is an extra demo about that (open the console, to see when i mean) https://codepen.io/utilitybend/pen/abxKpdN/5cd46990819b28e9ac6e5f88c2af73c4

@lukewarlow
Copy link
Collaborator

image

Okay I'm remembering all the jank with change and input events 😅

@lukewarlow lukewarlow added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Apr 16, 2024
@css-meeting-bot
Copy link

The Open UI Community Group just discussed [invokers] Should events trigger when invoking?, and agreed to the following:

  • RESOLVED: dispatch change events when an invoker changes an input value
The full IRC log of that discussion <hdv> brecht_dr: when I was creating a demo I noticed no event was fired when I added an invoker to step up and down in an input type=number
<hdv> brecht_dr: so I wondered if there was a reason for this, or if an event could be added for this?
<hdv> brecht_dr: the invoke event could work that we have in the experimental feature, that gets triggered before the new value is added
<hdv> brecht_dr: should the `change` event be triggered when using these buttons?
<keithamus> q+
<gregwhitworth> ack keithamus
<hdv> keithamus: I think lukew had a comment on this, I'll try to summon what he said
<hdv> keithamus: I think there's something around events firing at user gestures. So we could dispatch the change event only when it comes from a user gesture
<hdv> keithamus: so if you programatically click a button that invokes the input, the change event isn't fired, however if you're a user it wouldn't fire. That seems like a good middle ground to explore?
<gregwhitworth> ack dbaron
<hdv> dbaron: my intuition would be that the change event should fire if it changes from any source
<hdv> dbaron: does it fire a change evnt when value is explcitly changed
<brecht_dr> q+
<hdv> dbaron: ?
<gregwhitworth> ack brecht_dr
<hdv> keithamus: shakes no
<hdv> d/shakes no/*shakes no*
<hdv> brecht_dr: could we fire an afterinvoke event or would that make things even more complicated?
<hdv> keithamus: would be happy to explore dispatching a change event, but think there may get disagreement because of what's in the spec prose at the moment, which is written around what the user does
<hdv> keithamus: it's somewhat unprecedented to have this particular setup
<hdv> gregwhitworth: should we put forward a proposed resolution based on what the group wants? then maybe propose it in places?
<hdv> keithamus: we could explore what it looks like in chromium, but would probably get pushback?
<keithamus> PROPOSED RESOLUTION: explore dispatching change event based on invokers (regardless of user gesture)
<brecht_dr> +1
<dbaron> s/dbaron: ?/dbaron: I think the more indirect the source of the change, the more likely we want an event/
<keithamus> PROPOSED RESOLUTION: dispatch change events when an invoker changes an input value
<hdv> gregwhitworth: any objections to that proposed resolution?
<brecht_dr> +1 for me
<gregwhitworth> +1
<keithamus> RESOLVED: dispatch change events when an invoker changes an input value
<hdv> dbaron: I'm in favour, I think more events are better

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants