fix: add modalId prop for Modal

[RyanZhiNie]

Closes #771

📑 Description

Add the modalId prop to set the id attribute for the generated modal element.

Status

• Not Completed
• Completed

✅ Checks

• My pull request adheres to the code style of this project
• My code requires changes to the documentation
• I have updated the documentation as required
• I have checked the page with https://validator.unl.edu/
• All the tests have passed
• My pull request is based on the latest commit (not the npm version).

ℹ Additional Information

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
flowbite-svelte	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 21, 2023 8:31am
flowbite-svelte-update	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 21, 2023 8:31am

[shinokada]

It looks good to me.
@jjagielka What do you think?

[shinokada]

Thank you for the PR.
I modified a bit.

• use id instead of modalId
• add outsideclose prop to close a modal by clicking the outside modal
• dependencies updates
• workflow update
• etc.

[jjagielka]

Why did we need that id to be set? Original code has already <Frame rounded shadow {...$$restProps} class={frameClass}> and you were able to set id via $$restProps.

You have now introduce a static id called outsideModalId which is against the framework creation rules.

[shinokada]

You are right.
I put this issue because It had a hard-coded id.
We could use $$restProps in the first Frame for id.

As for the modal outside click functionality, I have a few reasons for implementing it:

• We have been receiving constant requests for the outside click function both here and in the Discord channel.
• It is not enabled by default, giving developers the flexibility to choose whether or not to use it.
• To close the modal, I added the id attribute with the value id="outerModal". I opted for an id since only one modal is open at a time, but using a class is also a viable option.

I understand your opinion against the outside click. But I would appreciate it if you could provide any suggestions for a better solution for closing a modal by outside click.

[jjagielka]

For the outside close it was as easy as adding one line to onAutoClose handler and no id settings was needed.

const onAutoClose = (e: MouseEvent) => {
    const target: Element = e.target as Element;
    if (autoclose && target?.tagName === 'BUTTON') hide(e); // close on any button click
    if (outsideclose && target === e.currentTarget) hide(e); // close on click outside
  };

[shinokada]

@jjagielka Thank you for your input. I just updated the Modal component.

[jjagielka]

@shinokada I'm sorry I've noticed a small issue with the solution proposed. That handler is conditionally connected:

on:click={autoclose ? onAutoClose : null}

we need to change it to:

on:click={onAutoClose}

or

on:click={autoclose || outsideclose ? onAutoClose : null}

[shinokada]

@jjagielka Done and thanks.