Lundalogik / lime-elements

Provides reusable web components for Lime CRM
https://lundalogik.github.io/lime-elements/versions/latest
Other
38 stars 12 forks source link

limel-dialog: dialog should be rendered in a portal #2893

Closed adrianschmidt closed 6 days ago

adrianschmidt commented 4 months ago

This issue was first reported as "when using a dialog inside another dialog, the whole dialog goes black". However, that is just a symptom of the underlying problem. The underlying problem is that the dialog created by limel-dialog is not rendered in a portal. It's frankly quite surprising that this hasn't been causing many problems, but I think the mitigating factor is that most dialogs is created via the Lime CRM platform's DialogRenderer, rather than by using limel-dialog directly, as using the DialogRenderer service means the Lime CRM Web Client will create the dialog at the top level of the app.

Below is the original description of this issue.

Current behavior

When using a dialog inside another dialog, the whole dialog goes black.

image

This isn't really a "normal" use case, but we have had a few instances of people having problems with it.

The cause of the problem, if I've understood it correctly, is the shadow that is used to indicate that the dialog content is overflowing, and that it's possible to scroll.

Reproduced in https://github.com/Lundalogik/lime-elements/pull/2600

Steps to reproduce the behavior:

import { Component, h, State } from '@stencil/core';
@Component({
    tag: 'limel-example-dialog',
    shadow: true,
})
export class DialogExample {
    @State()
    private isOpen = false;
    public render() {
        return [
            <limel-button
                primary={true}
                label="Open"
                onClick={this.openDialog}
            />,
            <limel-dialog open={this.isOpen} onClose={this.closeDialog}>
                <p>This is a simple alert-dialog.</p>
                <limel-dialog open={this.isOpen} onClose={this.closeDialog}>
                    <p>This is a dialog, inside another dialog!</p>
                    <limel-button
                        label="Oh no..."
                        onClick={this.closeDialog}
                        slot="button"
                    />
                </limel-dialog>
                ,
                <limel-button
                    label="Ok"
                    onClick={this.closeDialog}
                    slot="button"
                />
            </limel-dialog>,
        ];
    }
    private openDialog = () => {
        this.isOpen = true;
    };
    private closeDialog = () => {
        this.isOpen = false;
    };
}

Expected behavior

Both dialogs should render normally.

I thought we used limel-portal to render the dialog in, but it seems like we don't. I'm wondering if we shouldn't? If we did, the inner dialog wouldn't be rendered inside the outer dialog, so we should avoid this problem, I would think… 🤔

Environment

civing commented 3 months ago

@adrianschmidt could you please give it a more descriptive title?

Kiarokh commented 6 days ago

side note. I think the implementation of a nested modal (what I saw from consultants) was wrong anyways from the get go. One could place the trigger of a modal inside another modal perhaps. But they should not wrap the modal's code inside the first modal's code (like the example code in the issue ☝️ )... So this was no issue perhaps from the beginning, even thought the rendering bug in Chromium still exists.

Anyhow...