jarek-foksa / xel

Xel - Widget toolkit for building native-like Electron and Web apps
https://xel-toolkit.org
Other
677 stars 58 forks source link

x-input Uncaught TypeError #92

Closed DeezjaVu closed 4 years ago

DeezjaVu commented 4 years ago

With the following html form + x-input:

<form oninput="formInput(event)">
    <x-input name="frmTitle" type="text" required>
        <x-label>title</x-label>
    </x-input>
</form>

When selecting the x-input field and pressing the Arrow left or Arrow right keys I get the following error message:

x-input-uncaught-error

Any other keypress is fine. Only Arrow left/right triggers it.

Note that the <form> is inside a <Dialog>, which itself is inside a <x-button>.


Edit

Figured out that this only happens when the <dialog> is inside an <x-button> . Furthermore, when the dialog is open, it becomes a 'click' event trigger for the button.

Last but not least, when setting the dialog to not close when the backdrop is clicked, the whole page becomes a click event trigger for the x-button.

Taking the <dialog> out of the <x-button> - and for instance adding it to the document body dynamically - fixes the problem, but then it seems impossible to know when the dialog has been closed (finished its animation) as there doesn't seem to be an event dispatched when that occurs (unless I missed it).

jarek-foksa commented 4 years ago

When selecting the x-input field and pressing the Arrow left or Arrow right keys I get the following error message: (...)

This issue should be fixed in version 0.0.207.

Furthermore, when the dialog is open, it becomes a 'click' event trigger for the button.

Since the dialog is inside the button, a click event should propagate from the dialog to the button. You could use event.target to determine whether the dialog or the button was actually clicked.

Taking the

out of the - and for instance adding it to the document body dynamically - fixes the problem, but then it seems impossible to know when the dialog has been closed (finished its animation) as there doesn't seem to be an event dispatched when that occurs (unless I missed it).

Dialog should dispatch a custom userclose event when closed by user action. This event is unlike the standard close event which is dispatched even when the dialog was closed programmatically.

DeezjaVu commented 4 years ago

Thanks for the fix.

Since the dialog is a inside the button, a click event should propagate from the dialog to the button.

I disagree. It shouldn't. I understand that it does, as that's just how things work (html/javascript). But I disagree that it "should". Or maybe there should be an option to enable/disable this for the Dialog.

Dialog should dispatch a custom userclose event when closed by user action.

The 'userclose' event happens too soon. It's being dispatched before the animation ends. However, it looks like my initial thoughts regarding the timing of the 'close' event were wrong. It seems like that one is being dispatched after the animation finishes afterall.

jarek-foksa commented 4 years ago

I disagree. It shouldn't. I understand that it does, as that's just how things work (html/javascript). But I disagree that it "should". Or maybe there should be an option to enable/disable this for the Dialog.

By default the click event should bubble because this is how the DOM spec defines it. You could use dialog.addEventListener("click", (event) => event.stopPropagation()) to change that behavior.

The 'userclose' event happens too soon. It's being dispatched before the animation ends.

This behavior is intentional as animations belong to the presentation layer, while the open/closed state belongs to the data layer.

DeezjaVu commented 4 years ago

You could use dialog.addEventListener("click", (event) => event.stopPropagation()) to change that behavior.

From what I tried, that doesn't work, as the click event being triggered (the one defined on the x-button) is not the same as the one defined for the dialog. Meaning, event.stopPropagation() will prevent other listeners listening for the click event on the dialog to receive it.

The problem is the click event on the button is being triggered, which is a different event listener object.

This behavior is intentional

If it's intentional, then it is not consistent, as the 'userclose' and 'close' events are dispatched at a different time. One is before the animation, the other is after the animation.

Maybe to cover all bases, more event dispatching is required and in this order:

jarek-foksa commented 4 years ago

From what I tried, that doesn't work, as the click event being triggered (the one defined on the x-button) is not the same as the one defined for the dialog. Meaning, event.stopPropagation() will prevent other listeners listening for the click event on the dialog to receive it. (...) The problem is the click event on the button is being triggered, which is a different event listener object.

It works for me, are you sure you were using event.stopPropagation() rather than event.stopImmediatePropagation()? Also, make sure that useCapture in element.addEventListener("click", listener, useCapture) is not set to true.

If it's intentional, then it is not consistent, as the 'userclose' and 'close' events are dispatched at a different time. One is before the animation, the other is after the animation.

Right, it looks like a bug. I have opened a separate issue #93 to track it.

DeezjaVu commented 4 years ago

It works for me, are you sure you were using event.stopPropagation()

I was using all three: stopPropagation(), stopImmediatePropagation(), preventDefault().

But, I now tested it again, and you're right... it does work. So I must have messed up earlier somehow. So thanks for looking into this, much appreciated!!