ammarahm-ed / react-native-actions-sheet

A Cross Platform(Android, iOS & Web) ActionSheet with a flexible api, native performance and zero dependency code for react native. Create anything you want inside ActionSheet.
https://rnas.vercel.app
MIT License
1.42k stars 119 forks source link

Action sheet shadow is broken #277

Open mmmoussa opened 1 year ago

mmmoussa commented 1 year ago

The cause is here: https://github.com/ammarahm-ed/react-native-actions-sheet/commit/7fa623da8c7ff3e8e51a8cf68e8a4ff125b59aeb#r103324610. Unfortunately that commit does not provide any context for the change.

Setting overflow as 'visible' fixes the issue but I was hoping @ammarahm-ed could add more context as to why it was added in the first place in case reverting the change will break something else.

ammarahm-ed commented 1 year ago

Hey, where did you set the `overflow: visible prop?

mmmoussa commented 1 year ago

I replaced 'hidden' with 'visible' to get it working locally. If the overflow is hidden on the same node that has the shadow then the shadow will not be visible beyond that node's boundaries.

mmmoussa commented 1 year ago

Did some more testing and discovered one issue, which is that without setting overflow as hidden, rounded borders for the action sheet will get overflown by the sheet's top child element if it does not have the same rounding. I am achieving rounding with a shadow by making the overflow visible and also applying the rounding to the sheet's top child element. I think the ideal solution here is to allow the action sheet to take an overflow property specified within containerStyle and use it to override the overflow value when present.

ammarahm-ed commented 1 year ago

I think i fixed the overflow issue now by moving it into the inner container, can you try now with latest version

wilhelmeek commented 1 year ago

Hi @ammarahm-ed, with both positions of the overflow property it breaks implementations where there are externally-presented header elements, such as with our action sheet pictured here:

image

I'm happy to put together a PR to implement @mmmoussa's suggestion, but I'm keen to understand the need for the property in the first place. Cheers!