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

Implement Nimble dialog #378

Closed
gergo-papp opened this issue Feb 23, 2022 · 9 comments · Fixed by #658
Closed

Implement Nimble dialog #378

gergo-papp opened this issue Feb 23, 2022 · 9 comments · Fixed by #658
Assignees
Labels
client request Client team would immediately benefit from this change enhancement New feature or request

Comments

@gergo-papp
Copy link

😯 Problem to Solve

Several SystemLink web applications require a confirmation dialog (or generically just "dialog"). There is already a helium-uicomponents-based solution and an Angular Material-based one. None of these follow any design guidelines and none of them are theme-aware.

💁 Proposed Solution

Implement a nimble-dialog component and its appropriate Angular wrapper component by extending fast-dialog. This is similar to how the current nimble-drawer is implemented, but that component was specifically designed for "slide in" components that appear on the left or right side of the window.

The new component would appear in the middle of the window and would have the following sections:

  • header: the title of the dialog (this would usually be a question, like Are you sure you want to delete a file?
  • content slot: an optional content that allows any other text or component to be provided
  • footer: by default shows an OK and Cancel button (these would be customisable)

Other considerations - TBD

We could benefit from a programmatic way of opening a dialog by providing a NimbleDialogService that exposes functions for programmatically configuring and opening a dialog and then subscribing to it:

nimbleDialogService.openDialog('Are you sure you want to delete a file?').subscribe(confirmed: boolean => {
    if (confrmed) {
        deleteFile();
    }
});
@gergo-papp gergo-papp added enhancement New feature or request triage New issue that needs to be reviewed labels Feb 23, 2022
@gergo-papp
Copy link
Author

gergo-papp commented Feb 23, 2022

@jattasNI @rajsite I can't decide what would be the best approach for this low-level component in nimble. Having a programmatic approach (eg using a nimbleDialogService would make things very easy on the client side), but there is additional logic required to decide where to create the component in the DOM, since clients would want this to be "inside" a nimble-theme-provider.

Alternatively we can just use an ngIf in Angular, but then there it's harder to implement this behavior purely using services and observables.

Maybe we can have a hybrid, where we provide a very basic dialog in nimble, and implement a wrapper in systemlink-lib-angular that know where to dynamically create the component.

@gergo-papp
Copy link
Author

@fredvisser @jattasNI Is there any wild/rough estimate on when anyone could start working on this? I assume we should also involve POs and UX experts when creating a new component - AFAIK we do not have specs for a dialog and all the related behaviors

This is not super urgent so I don't want to pressure you but we want to know how much to invest in alternate short-term solutions

@jattasNI
Copy link
Contributor

@gergo-papp @fredvisser as we discussed in the UI WG today, we won't get to set the priority for this until next iteration at the earliest. To unblock your team you're welcome to temporarily use a different component.

@jattasNI jattasNI added the client request Client team would immediately benefit from this change label Feb 24, 2022
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Mar 1, 2022
@m-akinc
Copy link
Contributor

m-akinc commented Mar 1, 2022

Consider using dialog html element in template, possibly upstreaming to FAST.

@rajsite rajsite mentioned this issue Mar 8, 2022
10 tasks
@m-akinc m-akinc assigned m-akinc and unassigned m-akinc Mar 9, 2022
@m-akinc
Copy link
Contributor

m-akinc commented Mar 9, 2022

Needs some design/interactions guidance before we can really implement this. We presumably want nice support for common use cases like an info popup and a confirmation popup. But how much flexibility do we want to give, e.g. number, appearance, and labels of buttons? Do we want to support a fully customizable dialog, starting from a blank panel? UX/design may dictate that some of these things should stay constant/consistent. And the flexibility we expose to users will affect how we implement things, e.g. via slots or attributes.

@jattasNI jattasNI added the blocked Blocked on a third-party issue label Mar 9, 2022
@gergo-papp
Copy link
Author

From what I saw there is always a header/title, and a primary (and optionally secondary) buttons.

The content of the dialog can them be projected - eg a client might want to show a checkbox or display additional infromation in a paragraph

@TJ-G
Copy link

TJ-G commented May 9, 2022

Wanted to chime in with some feedback on requirements: I do think we'll want customization in the dialog. We would want the ability to adopt it in cases like the 'Calibration Policy' dialog we show on the assets page, where we need the user to provide more than just yes/no information:
image

@m-akinc m-akinc self-assigned this Jun 24, 2022
@TrevorKarjanis
Copy link
Contributor

TrevorKarjanis commented Jun 24, 2022

@jattasNI @rajsite I can't decide what would be the best approach for this low-level component in nimble. Having a programmatic approach (eg using a nimbleDialogService would make things very easy on the client side), but there is additional logic required to decide where to create the component in the DOM, since clients would want this to be "inside" a nimble-theme-provider.

Alternatively we can just use an ngIf in Angular, but then there it's harder to implement this behavior purely using services and observables.

Maybe we can have a hybrid, where we provide a very basic dialog in nimble, and implement a wrapper in systemlink-lib-angular that know where to dynamically create the component.

I believe the solution for this, as used by AM experimental (MDC), is portal outlets. Angular components work with HTML slots, so a portal is used to dynamically inject an Angular component into the dialog slot(s). We'll see what Mert's PR produces, but I've worked with portals extensively and can probably create the service if need be whether it lives in Nimble or the shared lib. I think portals use ViewContainerRef.createComponent, so that's also probably an option as well.

@rajsite rajsite mentioned this issue Jul 12, 2022
1 task
@m-akinc m-akinc removed the blocked Blocked on a third-party issue label Aug 3, 2022
@nate-ni nate-ni moved this to 🏗 In progress in Nimble Design System Priorities Aug 5, 2022
@mollykreis
Copy link
Contributor

@m-akinc, shouldn't this issue stay open until we have Angular & Blazor support for the dialog (assuming we don't have a different issue for adding that support)?

mollykreis pushed a commit that referenced this issue Aug 12, 2022
## 🤨 Rationale

Fixes #378 

## 👩‍💻 Implementation

Refer to spec document.

## 🧪 Testing

Created automated tests. Could not come up with a good way to test `prevent-dismiss` attribute.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or determined no changes are needed.

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
@nate-ni nate-ni moved this from 🏗 In progress to ✅ Done in Nimble Design System Priorities Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client request Client team would immediately benefit from this change enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

6 participants