openui / open-ui

Maintain an open standard for UI and promote its adherence and adoption.
https://open-ui.org
Other
3.59k stars 191 forks source link

[invokers] Should events trigger when invoking? #1033

Open brechtDR opened 7 months ago

brechtDR commented 7 months ago

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

mfreed7 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

image

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

css-meeting-bot commented 7 months ago

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

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
github-actions[bot] commented 1 month ago

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.