Simplify Svelte Modal component / usage

See comment in MR. It should either be confirmed it's Ok to remove the parent condition, or limit the changes to just removing the condition as described in the issue summary.

This popup code seems to be inspired from https://svelte.dev/repl/033e824fad0a4e34907666e7196caec4?version=3.20.1 and may require more refactoring or can be replaced with some other simple popup.

OK so after some digging it seems like the reason for all this is because you can pass in all these parameters through props, i.e. whether or not to have a closeButton at all, for example. So, to that end:

It's possible {#if state.closeButton} is not needed either, but we should get some confirmation of that before going ahead with removing it.

It's "needed" in the sense that if we were to have a second modal where we didn't want a closeButton, it could be useful to be able to pass it in as a prop. Leaving that piece in allows us to do that.

In terms of this issue, I think it addresses the primary current concern, but that doesn't really move the needle.

I also see the following which may also want to come out?
https://git.drupalcode.org/project/project_browser/-/blob/b5a813e9bee97a...

    if (isFunction(show)) {
      open(show);
    } else {
      close();
    }

The purpose of the isFunction is that you could declare these individual pieces, ex.g. the close button, as Components in and of themselves, not relying on the default one.

That totally seems like overkill to me. So I land where narendraR did... which I think is that this Modal is probably unnecessarily complicated because it was brought in from boilerplate, and I think this could be simplified quite a bit, or we could pull in some other Modal (is there anything in core/Claro?).

Updating issue summary to reflect.

Good observations! We could feasibly use the existing Drupal.dialog library because the contents of the modal don't need to interact with the rest of the app. It also happens to be a very accessible dialog, so that would address several concerns we'd otherwise have with the current modal.

It would have to work a little differently, but if you add core/drupal.dialog as a dependency, this is roughly what would be needed in the callback that is invoked when someone clicks "download"

• Create a new DOM element with a unique id that has the contents of what should appear in the modal. It won't be a svelte component, just a plain old dom element
• Drupal.dialog(document.querySelector('#the-id-of-the-element-created-in-prior-step')).showModal()

When the modal closes ,it automatically removes the modal content DOM element, so you could re-use the same ID, and would not have to worry about a bunch of visually hidden former modals stacking up.

Used Drupal.dialog and removed svelte modal.

The code is largely simplified with this new implementation and works as expected (has enhanced its accessibility rather).
The one thing that is missing is that the background scrolling on the page should be disabled when a modal/dialog is opened. Marking NW for this one change, everything else is good to go otherwise.

Very good point @srishtiiee.
A discussion is going around it at https://www.drupal.org/project/drupal/issues/2707291 and may be we don't need to do it explicitly here.

We definitely don't need to address the scrolling, that's a drupal.dialog issue and #2707291: Disable body-level scrolling when a dialog is open as a modal is the proper home for it. Project Browser doesn't need to figure that one out.

Spotted one thing in the MR that I'd like changed so setting to NW, but overall looks good. Also rerolled to make things a little easier.

Looks good! Test are running again because I rebased, but it'll switch out of the RTBC if for some reason the tests stop passing.

Merged, thanks!