Skip to content

allow to specify the backdrop parent for the modal component#35634

Closed
Johann-S wants to merge 1 commit intomainfrom
jo-feat-modal-parent-backdrop
Closed

allow to specify the backdrop parent for the modal component#35634
Johann-S wants to merge 1 commit intomainfrom
jo-feat-modal-parent-backdrop

Conversation

@Johann-S
Copy link
Member

@Johann-S Johann-S commented Jan 2, 2022

Hi everybody!
I hope you're doing well, I wish you the best for this new year!

A small PR, to allow to specify the root element for the modal backdrop, it can be useful for users who use Bootstrap with Gatsby for example

Edit: The build failed on something I didn't changed, that's strange 🤔

@Johann-S Johann-S requested a review from a team as a code owner January 2, 2022 14:15
@Johann-S Johann-S requested a review from XhmikosR January 2, 2022 14:37
@GeoSot
Copy link
Member

GeoSot commented Jan 4, 2022

Hello @Johann-S 😃

Can you please help me to understand how can it help Gatsby users?

We had another PR trying to introduce a modal option, defining rootParent and inherit it to backdrop. I believe this option would be more handy, without exposing backdrop rootParent option.

In case I am right and we provide the modal rootParent option, does this PR add any value to functionality?

Ps: would appreciate your help/response on #34989 (comment)

@Johann-S
Copy link
Member Author

Johann-S commented Jan 4, 2022

Hey :)

Finally after some tests on my project, it's because of purgeCSS remove .modal-backdrop added by our JS.
So feel free to close that PR if it's not useful 👌

@XhmikosR
Copy link
Member

XhmikosR commented Jan 5, 2022

So, do we need this after all?

@Johann-S
Copy link
Member Author

Johann-S commented Jan 5, 2022

Nope I don't think it's useful at all 😆

@XhmikosR XhmikosR closed this Jan 6, 2022
@XhmikosR XhmikosR deleted the jo-feat-modal-parent-backdrop branch January 6, 2022 23:15
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.

3 participants