search

doomsower / react-native-modal-popover

React-Native pure JS popover that uses Modal
MIT License
322 stars 45 forks source link

Making the background overlay optional #26

Closed AdamGerthel closed 6 years ago

AdamGerthel commented 6 years ago

I have a use-case where I want to use a small close button (like an "x") in the top right corner of the popover, instead of using the overlay to close. I can add the button, but I can't get rid of the overlay.

I propose to make it optional to use the overlay (using a prop on Popover). I've taken a look at the source code, and could probably provide a PR for this. But do you think it will work, and is it a "general" enough feature to add?

doomsower commented 6 years ago

I think it’s good idea. However, I won’t be able to review/merge the PR until 25th, as I’m traveling.

Konstantin Kuznetsov

On 18 Jun 2018, at 09:51, Adam Gerthel notifications@github.com wrote:

I have a use-case where I want to use a small close button (like an "x") in the top right corner of the popover, instead of using the overlay to close. I can add the button, but I can't get rid of the overlay.

I've taken a look at the source code, and could probably provide a PR for this. But do you think it will work, and is it a "general" enough feature to add?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

AdamGerthel commented 6 years ago

Ok, that's not a problem. I'll try to do the PR in the next couple of days.

AdamGerthel commented 6 years ago

I have made a working fork (https://github.com/AdamGerthel/react-native-modal-popover) with this feature now. But I realised that there might be a problem: Even though the overlay isn't rendered, it's still not possible to interact with the underlying elements. When there is no visible overlay, I think a user would expect to be able to do that (and it's also what I wanted to achieve).

I'm guessing this is due to the Modal component? If so - do you know of any way around this? I tried removing it, entirely but that seems to break the positioning (my sample popover doesn’t display at all without the Modal component).

doomsower commented 6 years ago

If there is no way to make modal touch-transparent, then it should be replaced with a View. But this would require provider, because view obeys view hierarchy. I’m not sure that I want to remove modal from the module that has modal in its name

Konstantin Kuznetsov

On 21 Jun 2018, at 00:08, Adam Gerthel notifications@github.com wrote:

I have made a working fork (https://github.com/AdamGerthel/react-native-modal-popover) with this feature now. But, I realised that there might be a problem. Even though the overlay isn't rendered, it's still not possible to interactive with the underlying elements. When there is no visible overlay, I think a user would expect to be able to do that (and it's also what I wanted to achieve).

I'm guessing this is due to the Modal component? If so - do you know of any way around this? I tried removing it, entirely but that seems to break the positioning (my sample popover doesn’t display at all without the Modal component).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

AdamGerthel commented 6 years ago

Agreed. And I have a feeling it would require a lot of work to make it work without using Modal.

My fork is functional but doesn't add anything, because if the Modal needs to be there - then removing the overlay doesn't make much difference anyway. And if someone wants to make the overlay invisible then it's already possible using backgroundStyle.

Closing.