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

Remove toasts from DOM after hiding them #27807

Closed
MartijnCuppens opened this issue Dec 10, 2018 · 5 comments
Closed

Remove toasts from DOM after hiding them #27807

MartijnCuppens opened this issue Dec 10, 2018 · 5 comments
Labels

Comments

@MartijnCuppens
Copy link
Member

From #27792 (comment):
We also do this with the alerts. The problem with not removing them is that there's a margin above the second toast if we dismiss the first: https://deploy-preview-27792--twbs-bootstrap4.netlify.com/docs/4.1/components/toasts/#stacking

@Johann-S
Copy link
Member

Shouldn't we change the selector by something like that: .toast:not(.hide)+.toast ?

Because if folks wants to keep using the same toast, they have to add new toast to their DOM everytime 😟

/CC @XhmikosR

@MartijnCuppens
Copy link
Member Author

Also fine by me, but I would prefer the same approach for alerts and toasts unless there's a reason why we use different approaches.

@Johann-S
Copy link
Member

Johann-S commented Dec 10, 2018

I don't know why we choose to remove the alert from the DOM, but I'm not a big fan of removing things from the DOM...
I wasn't there when this decision was made 😆 maybe @XhmikosR have some information why they chose that path ?

@XhmikosR
Copy link
Member

XhmikosR commented Dec 10, 2018

Well, the idea is that if we are going to reuse an element, we should just hide it, otherwise remove it.

On the other hand, hiding/showing should be faster than adding/removing...

@Johann-S
Copy link
Member

A toast should be reusable, so I think the CSS solution suits better here

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

No branches or pull requests

3 participants