Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new modal option closeAll #1040

Closed
IlyaSemenov opened this issue Aug 22, 2024 · 2 comments
Closed

Add new modal option closeAll #1040

IlyaSemenov opened this issue Aug 22, 2024 · 2 comments
Labels
wontfix This will not be worked on

Comments

@IlyaSemenov
Copy link
Contributor

Description

As a developer, I often need to switch between modals, such as when I have Sign In modal with a button to open Sign Up modal and vice versa. However, oruga.modal.open() always opens the new modal on top of the old modal.

I end up using this composable:

export function useOpenModal() {
  const oruga = useOruga()

  return function openModal(params: ModalProgrammaticProps & { closeAll?: boolean }) {
    const { closeAll = true } = params
    if (closeAll) {
      oruga.modal.closeAll()
    }
    return oruga.modal.open(params)
  }
}

and then:

const openModal = useOpenModal()

openModal({ component: AuthSignupModalContent })

I propose to add a new flag/option to OrugaOptions: modal: { closeAll?: boolean }, available as global config option, component option, and programmatic open option.

Why Oruga need this feature

This will reduce boilerplate/simplify modals flow for many projects. Arguably, closeAll more often than not will be enabled by default by the developers and only explicitly disabled in few cases when the UX flow involves nested modals.

@IlyaSemenov
Copy link
Contributor Author

Similarly, there could be closeActive option (perhaps only for oruga.modal.open() and not for the declarative setup) that will allow to close the topmost modal but keep the upper layered modals. The example use case will be:

  • There is Auth Modal which presents two buttons: Sign In and Sign Up, both opening the respective nested modal.
  • Sign In modal allows to switch to Sign Up modal and vice versa. closeAll will not work here because the upper Auth Modal should stay, only the nested modals should replace each other.

I suppose that is currently possible by emitting close before opening the new modal, but that requires a lot of boilerplate in each component. It can't be decomposed into a composable similarly to the above. Thus, the new option will be very helpful.

@mlmoravek
Copy link
Member

Hey @IlyaSemenov, with #1058 I reimplemented the programmatic interfaces of the relevant components.
I don't see the purpose of adding closeAll property to the open() interface. I think those things are to specialist application behaviors and can also just called before open a new one.

But I added a close() interface that closes the last open programmatic component in the instance registry stack.

@mlmoravek mlmoravek added the wontfix This will not be worked on label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants