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

md-icon in md-dialog can not show up if with bootstrap 5 #5182

Closed
Staler2019 opened this issue Nov 10, 2023 · 7 comments · Fixed by #5184
Closed

md-icon in md-dialog can not show up if with bootstrap 5 #5182

Staler2019 opened this issue Nov 10, 2023 · 7 comments · Fixed by #5184

Comments

@Staler2019
Copy link
Contributor

Staler2019 commented Nov 10, 2023

What is affected?

Component

Description

I want to add delete icon at top of dialog.

By following the demo on website, the icon didn't show up.

Here's website's demo code snap and its demo pic:

<md-dialog style="max-width: 320px;">
        <div slot="headline">Permanently delete?</div>
        <md-icon slot="icon">delete_outline</md-icon>
        <form id="form" slot="content" method="dialog">
          Deleting the selected photos will also remove them from all synced
          devices.
        </form>
        <div slot="actions">
          <md-text-button form="form" value="delete">Delete</md-text-button>
          <md-filled-tonal-button form="form" value="cancel"
              autofocus>Cancel</md-filled-tonal-button>
        </div>
</md-dialog>

Screenshot 2023-11-10 at 11 22 35

Reproduction

lit playground

Workaround

I haven't found a workaround. But to found out when disabling bootstrap, it works.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

Failing in 1.0.1

Browser/OS/Node environment

Browser: Firefox 119.0.1, Chrome 119.0.6045.123
OS: macOS 14.1 (m2)
Bootstrap: 5.0.2

@Staler2019 Staler2019 changed the title Icons can not show up if added in dialog with bootstrap 5 md-icon in md-dialog can not show up if with bootstrap 5 Nov 10, 2023
@christophe-g
Copy link
Contributor

This seems to be due to the quite aggressive css rebooting of bootstrap. In particular, this rule:

*, ::after, ::before {
    box-sizing: border-box;
}

It alters the positioning of the icon, which is then hidden behind the dialog header.

@Staler2019
Copy link
Contributor Author

I'm a newborn in font-end works and I just know basics.

@christophe-g could you advice me some solutions or searching directions for solving this problem?
And I'm working with a .net Core 6 MVC project, thanks.

@christophe-g
Copy link
Contributor

A very quick / dirty patch for this case could to reset the default box-sizing value for all components where this is a problem :

md-icon {
    box-sizing: initial;
}

Ideally, use as less dependencies as possible (jquery/bootstrap) ; )

@asyncLiz
Copy link
Collaborator

I think this might be a subtle bug. The icon has 24px of top padding added to it from the dialog, but I think that should actually be margin-top.

The space being added by the dialog to the icon is intended to be external space, not internal space.

@Staler2019
Copy link
Contributor Author

A very quick / dirty patch for this case could to reset the default box-sizing value for all components where this is a problem :

md-icon {
    box-sizing: initial;
}

Ideally, use as less dependencies as possible (jquery/bootstrap) ; )

I'll give it a try on workdays, thank you.

@Staler2019
Copy link
Contributor Author

I think this might be a subtle bug. The icon has 24px of top padding added to it from the dialog, but I think that should actually be margin-top.

The space being added by the dialog to the icon is intended to be external space, not internal space.

That's a great idea. After changing what you have said, it works.

I might make a PR later.

@Staler2019
Copy link
Contributor Author

I'm unable to submit a PR at the moment, so I've made adjustments to my code in this way.

md-dialog > md-icon {
    padding-top: initial;
    margin-top: 24px;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants