-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
layer clickaway option for render-to-layer/popover #2359
Conversation
How can I test this? |
So on icon menu/menu - you can still click underlying elements, because they pass in On Popover, it doesn't, so if you click on the left nav, it will now close the popover rather than trigger a navigation. |
this._layer.style.bottom = 0; | ||
this._layer.style.left = 0; | ||
this._layer.style.right = 0; | ||
this._layer.style.zIndex = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20
seems to be a magic number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it matches zIndex of Popover L#124 - what do you want to do with it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zIndex of 0 works, so now using this
What should we do for https://github.com/callemall/material-ui/blob/master/src/dialog.jsx#L458? |
@chrismcv I like this new property 👍. |
@oliviertassinari - this should be good now? |
What should we do with the |
Because it uses Overlay, I think this should probably remain as is - as the Overlay is on top of the _layer, so will have click precedence. |
@oliviertassinari new commit with zIndex on theme |
@@ -118,7 +132,7 @@ const Popover = React.createClass({ | |||
position: 'fixed', | |||
top: anchor.top, | |||
left: anchor.left, | |||
zIndex: 20, | |||
zIndex: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we use layerZIndex
?
@oliviertassinari - this looks good now |
@chrismcv Looks good 👍. Could you squash down? I will merge 😄. |
976ce86
to
1141870
Compare
layer clickaway option for render-to-layer/popover
Thanks |
and you @oliviertassinari |
@oliviertassinari @chrismcv
|
You are right on both points. |
@oliviertassinari - ended up changing the property name to
useLayerForClickAway