Confirm Modal should submit on Enter key pressed

[ibilux]

Confirm Modal should submit true value on Enter key pressed (like closing on Escape key pressed).
We should change button type from type="button" to type="submit".
Or use HTML Form Element <form>.

https://github.com/skeletonlabs/skeleton/blob/fa2af91a23e37a92ddf820c578ffaab135766a99/packages/skeleton/src/lib/utilities/Modal/Modal.svelte#L243C8-L243C8

[endigo9740]

@ibilux can you link to the source on this please. We follow the ARIA APG guidelines on these sorts of issues to meet a11y concerns:
https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/

It may be in there, but they cover a lot so just let me know where specifically.

[riziles]

@ibilux , I found a good discussion here: https://ux.stackexchange.com/questions/130704/enter-key-action-on-modal-dialogs

They assert that the enter key should do something, but if "submit" = fire nukes, than the default should probably be "cancel".

[endigo9740]

That generally meets my expectations as well, but some of this may be a limitation for our focus trap implementation. What I'm leaning towards is some ways for users to opt into and configuration this behavior in some fashion. Rather than just hardcoded on-by-default. Defaulting to this may not meet everyone's expectations and it may violate some a11y concerns.

[endigo9740]

Taking a deeper look at this - one option we have here is the autofocus attribute. The MDN docs even point out this specific use case for the Dialog element:

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/autofocus

The autofocus global attribute is a Boolean attribute indicating that an element should be focused on page load, or when the that it is part of is displayed.

This doesn't introduce the complexity or lead the possibility of side effects that a form and button type can lead to. However, SvelteKit provides warning against this:

In this case I think they are technically correct, this should be used sparingly, but for our use case specifically I think this is the correct option. If so, we'll need to implement this and append a one-off comment to disable this warning like so:

<!-- svelte-ignore a11y-autofocus -->
<button type="button" class="btn {buttonPositive}" on:click={onConfirm} autofocus>{buttonTextConfirm}</button>

However, we should likely take the UX concerns into play here - that the positive action may not always be the desired focus element. In that case we will need a prop like export let autofocus = 'positive' to adjust focus, but only allow one option at a time.

<button type="button" class="btn {buttonNeutral}" on:click={onClose} autofocus={autofocus === 'neutral'}>
    {buttonTextCancel}
</button>
<button type="button" class="btn {buttonPositive}" on:click={onConfirm} autofocus={autofocus === 'positive'}>
    {buttonTextConfirm}
</button>

I'm not sure I like the complexity of this though, especially given trying to balance the component props settings versus dynamic parameters passed for one-offs.

@ibilux @riziles thoughts?

[ibilux]

@endigo9740 the autofocus attribute doesn't work well in SvelteKit, in some cases it won't work, or it will work only for the first time you call/import the component.
I'm using another working solution using the use directive :

<script lang="ts">
  import { tick } from "svelte";
  async function autofocus(node: HTMLElement){
    await tick();
    node.focus();
  }
</script>

<button type="button" class="btn {buttonPositive}" on:click={onConfirm} use:autofocus>
    {buttonTextConfirm}
</button>

However, we should likely take the UX concerns into play here - that the positive action may not always be the desired focus element. In that case we will need a prop like export let autofocus = 'positive' to adjust focus, but only allow one option at a time.

You are right about the UX.

[Sarenor]

I'd certainly ask for a configurable prop, probably in the modalSettings object for it.

Interesting questions is if it has 2 or 3 allowed values: positive|negative|none.
Currently, none would be the correct default without changing existing behaviour.

The behaviour would be better a11y though, so I think I'd rather have just positive|negative with positive being the default.

[endigo9740]

the autofocus attribute doesn't work well in SvelteKit, in some cases it won't work, or it will work only for the first time you call/import the component.

It does work, I tested a working example of it yesterday. SvelteKit can't disable a core browser feature like that. Given the modal component is added and removed from the DOM when a modal is opened or closed, it does work repeatedly.

Though you do bring up a good use case for testing multiple confirm prompts back to back. That's an edge case, but one that should be tested

I'd certainly ask for a configurable prop, probably in the modalSettings object for it.

That's where the complexity comes in. I have an idea of how to solve this, but I'll have to investigate.

But either way, I don't think the current PR will be our solution. It doesn't account for this and is not nearly as configurable IMO.

[endigo9740]

In an effort to prepare for Skeleton v3, we're consolidate some related issues down to a single ticket. This will ensure that we can see the full context of requests when the time comes to refactor and update this feature going forward. If you wish to add additional feedback or suggestions, please so here:

• NEXT: Overlay System (modals/drawers/toasts) #2357