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

Variabilize backdrop class name (#34094) #34149

Merged
merged 5 commits into from
Jun 25, 2021
Merged

Variabilize backdrop class name (#34094) #34149

merged 5 commits into from
Jun 25, 2021

Conversation

romdum
Copy link
Contributor

@romdum romdum commented May 31, 2021

To resolve #34094, create a new class offcanvas-backdrop which extends modal-backdrop. It helps avoid confusion if an offcanvas and a modal is open and two backdrop are shown.

@GeoSot
Copy link
Member

GeoSot commented Jun 22, 2021

@romdum I did some cleanup in order to help this on track. I hope you don't mind

@twbs/css-review any help on the sccs?

The main cause for now is just to extend the modal-backdrop class, use it on offcanvas-backdrop, (initial idea)

In order to avoid duplication, one Idea is to use a new file backdrop.scss create a new class backdrop and use it to both as extend

romdum and others added 5 commits June 25, 2021 01:01
- Update modal and offcanvas backdrops to use the mixin (placeholder wouldn't work with params I believe)
- Plus, modals and offcanvases should be able to be used together, so separate values for z-index (already implemented) are reflected properly here.
- Offcanvas still extends modals variables by default, save for z-index
@GeoSot
Copy link
Member

GeoSot commented Jun 24, 2021

Js scoped, seems ok

@mdo
Copy link
Member

mdo commented Jun 25, 2021

Thanks for the fix lol, I must have been braindead earlier. I pushed another change to fix our z-indexes. I'll make sure to cover this in our highlights for the blog post btw. New z-indexes make more sense now IMO.

@mdo mdo merged commit bbd87ca into twbs:main Jun 25, 2021
@mdo
Copy link
Member

mdo commented Jun 25, 2021

I'm on airplane wifi and I screwed up the push, so this was merged to main. Working to at least squash those commits in main and we can iterate in subsequent PRs if needed. Sorry about that folks!

1 similar comment
@mdo
Copy link
Member

mdo commented Jun 25, 2021

I'm on airplane wifi and I screwed up the push, so this was merged to main. Working to at least squash those commits in main and we can iterate in subsequent PRs if needed. Sorry about that folks!

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

Successfully merging this pull request may close these issues.

v5 backdrop toggle naming offcanvas / modal
3 participants