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

Please provide a ModalService that properly encapsulates the opened modal #1715

Open
2 tasks done
mak-dunkelziffer opened this issue Feb 27, 2025 · 0 comments
Open
2 tasks done
Labels
triage We discuss this topic in our internal weekly type: enhancement New feature or request

Comments

@mak-dunkelziffer
Copy link

Prerequisites

  • I have read the Contributing Guidelines.
  • I have not leaked any internal/restricted information like screenshots, videos, code snippets, links etc.

Suggestion / feature request

The current implementation of the ModalService doesn't properly encapsulate the opened modal. It's more of a ModalBuilder that puts the modal into the DOM for you, but doesn't take care of the remaining life cycle.

Why is this an issue:

  • MINOR: This broken encapsulation makes testing a pain, because it's not enough to mock the ModalService. You also have to mock the IxActiveModal in a compatible manner including the registering and invocation of callbacks
  • MAJOR: There is no easy way to close all opened modals, e.g. in an HTTP interceptor that redirects to /500. It would be a huge improvement to be able to write inject(ModalService).close() anywhere to close any opened modals.

Potential interface for a ModalService that works as a global Singleton and assumes "no stacked modals":

@Injectable({
  providedIn: "root"
})
export class ModalService {
  private activeModal

  open(content: <ComponentWhichImplementsNecessaryInterface>, data: { ... }, ...) {
    // close/dismiss other opened modals
    // open new modal in the same manner as the current ModalService does
    // store reference in activeModal
  }

  close() {
    // close/dismiss opened modal
  }

  events() {
    // provide a common interface to listen to modal events
    // e.g. for Angular an RxJS observable, which "completes" as soon as the modal is closed
  }
}

A more sophisticated ModalService e.g. for stacked modals could be provided as required, but as overusing modals isn't the best decision for accessibility reasons anyways, not providing an API for stacked modals could also be a conscious decision to discourage such UI.


Angular 19.0.7
Siemens IX 2.6.1

@mak-dunkelziffer mak-dunkelziffer added triage We discuss this topic in our internal weekly type: enhancement New feature or request labels Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage We discuss this topic in our internal weekly type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant