search

masonitedoors / MDL

Masonite Design Language components
https://masonitedoors.github.io/MDL
Apache License 2.0
5 stars 4 forks source link

ModalBackdrop not honoring its "show" prop #131

Open josephfusco opened 4 years ago

josephfusco commented 4 years ago

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

When the show prop value is true, <ModalBackdrop show={true} />, the component is working as expected showing both the backdrop and children.

Screen Shot 2020-09-03 at 4 00 30 PM

When the show prop value is false, <ModalBackdrop show={false} />, the component hides the children content but is still showing the overlay.

Screen Shot 2020-09-03 at 4 00 07 PM

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

https://codesandbox.io/s/nervous-cohen-tdzb3?file=/src/index.js

What is the expected behavior?

When the show prop is false, no part of the component should be visible or obstructing clicks.

Which versions of MDL, and which browser / OS are affected by this issue? Did this work in previous versions of MDL?

This is an issue in the latest version of MDL, 2.24.1. I am not sure how far back this issue goes but know that this is also not working in 2.23.0.

tarikhamilton commented 4 years ago

Modal technically refers to the content, which is why I called it ModalBackdrop, but that is the right component to use.

I modified your codesandbox for it to work. https://codesandbox.io/s/vigorous-wind-3rhdg It works fine if you conditionally render it. That's how I've used it in other apps, so that could be a workaround for now.

But "show" prop should reveal/hide it. I'm pretty sure that was the intent. If "show" is fixed, there should be a quick fade-out. If you look at the source, there's a transition for the modal to fade in, then another for the actual content. We want to preserve that going reverse. Conditionally rendering messes up the modal fade-in and fade-out.