[listbox|select-rich|combobox] select-rich model-value-changed dispatched on keyboard navigation

[MathieuPuech]

Describe the bug

When you select a value with keyboard, the model-value-changed event is dispatched before the user validate the selection.

On native select, the change event is dispatched only when Enter key is pressed.

I saw an error on the documentation too: instead of model-value-changed event, it's select-model-value-changed (see https://lion-web-components.netlify.app/?path=/docs/forms-select-rich--render-options)

To Reproduce

Steps to reproduce the behavior:

1. Go to https://lion-web-components.netlify.app/iframe.html?id=forms-select-rich--default-story&viewMode=story
2. Add a event listener on lion-select-rich document.querySelector('lion-select-rich').addEventListener('model-value-changed', console.log)
3. Click on lion-select-rich
4. Use down key

Expected behavior

Event should not be dispatched before the value is selected

[tlouisse]

By default, it mimics the platform interaction (of <select>). On windows/linux, selection happens on keydown. On mac, you need to confirm explicitly:
https://lion-web-components.netlify.app/?path=/docs/forms-select-rich--interaction-mode

On which platform did you test? If it is on windows/linux, it is expected behavior (you see the invoker button updating its state as well)

[MathieuPuech]

I'm testing on a windows for the event propagation.

On windows, the event is dispatched when you press Enter even if the value changed in the display

[tlouisse]

Fair point.
It has to be said though that model-value-changed !≈≈ change.
For instance, in a regular <input type="text"> change will only fire on blur (and if the value has changed after entering the field).
In that regard, model-value-changed is more simular to the native input event.

I will later spin up a vm to do a test with a native select on windows and see if we're doing the right thing here :)

[tlouisse]

Tested it. You're right about an opened select.

What happens:

• closed select and key/up down will update the invoker and change select.value
• opened select and key up/down will update the invoker, but NOT change select.value

Currently, we also sync an opened select to the invoker realtime, hence the model-value-changed event.
This syncing of values should not happen once opened.

I will mark this as a bug/improvement

[ffoodd]

May I ask if that's still a concern? Is there any workaround I can apply, as a subclasser?

Thanks!

[MathieuPuech]

there issue is still here, in our subclass, we have added a change event that is dispatched only when it should:

  setCheckedIndex(index) {
    super.setCheckedIndex(index);
    if (this.formElements[index] && this.__oldIndex !== index) {
      if (!this.opened) {
        this.__oldIndex = index;
        this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
      }
    }
  }

[tlouisse]

@MathieuPuech Nice 👍 We could incorporate this into our core, so that we get similar behavior for all listboxes(listbox/combobox/select-rich) and choice groups in general (meaning radio-groups/checkbox-groups) as well.

To make it work for all those components, we could check for focused instead of opened and then add some meta data to the model-value-changed event, which should be the single source of truth where other values and events could be derived from: https://lion-web.netlify.app/docs/systems/form/model-value/#event-meta

We could add something like isTriggeredAfterBlur here.

@ffoodd @MathieuPuech Then you could make a generic mixin for your extension layer (N.B. this is a proposal, not implemented yet):

const ExtionsionLayerFormControlMixin = (superclass) =>  ExtionsionLayerFormControl extends superclass {
   constructor() {
     super();
     
     this.addEventListener('model-value-changed', (ev) => {
       if (ev.detail.isTriggeredAfterBlur) {
         this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
       }
     });
   }
}

Or we could consider to add this by default in v1 as well for better DX:

<form-control @change="${onChangeAfterBlur}" @input="${onUserInput}" @value="${onProgrammaticChange}">

[tlouisse]

Related discussion: #24