Skip to content
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

feat: added bottom inset #237

Merged
merged 14 commits into from
Feb 9, 2021
Merged

feat: added bottom inset #237

merged 14 commits into from
Feb 9, 2021

Conversation

DawidVanderhoven-TomTom

I am very new to reanimated so this may be a completely incorrect way of approaching this, however this is my attempt so far. This allows you to create space below the bottom sheet in case of a situation where a footer or a bottom menu may need to be present.

This resolves #95

@ferrannp
Copy link

@DawidVanderhoven-TomTom what about for v3? Any ideas?

@DawidVanderhoven-TomTom
Copy link
Author

@ferrannp I am using v2 at the moment but perhaps if you attempt to do something similar on here subtracting the amount of space you want below would be worth a shot however this is untested and I am still not totally sure if this is the correct approach.

vshia pushed a commit to humandx/react-native-bottom-sheet that referenced this pull request Jan 29, 2021
@gorhom
Copy link
Owner

gorhom commented Feb 1, 2021

thanks @DawidVanderhoven-TomTom for submitting this pr, i will review it shortly

@DawidVanderhoven-TomTom
Copy link
Author

DawidVanderhoven-TomTom commented Feb 4, 2021

@gorhom I have now also provided an example and added the safeHandleHeight to the handleClose as I found that I could sometimes see the handle when stacking modals when using it with bottomInset.

@gorhom
Copy link
Owner

gorhom commented Feb 8, 2021

hi @DawidVanderhoven-TomTom , i have tested you pr , the example that you added works fine , but other modal examples got broken.

i understand the need for this, and i would suggest to change the approach a bit,

you could pass topInset & bottomInset to BottomSheetContainer and set them as the root container top and bottom like :

const containerStyle = useMemo(
  () => [
    styles.container,
    {
      top: topInset,
      bottom: bottomInset,
      overflow: 'hidden',
    },
  ],
  [bottomInset, topInset]
);

and it should achieve the goal 👏

@gorhom
Copy link
Owner

gorhom commented Feb 8, 2021

the solution above could be implemented easily in v3 too

@DawidVanderhoven-TomTom
Copy link
Author

Thanks @gorhom!

@gorhom gorhom changed the title Bottom Inset feat: added bottom inset Feb 9, 2021
@gorhom gorhom merged commit ed8b86b into gorhom:master Feb 9, 2021
@gorhom
Copy link
Owner

gorhom commented Feb 9, 2021

thanks @DawidVanderhoven-TomTom for submitting this pr <3

@ferrannp
Copy link

@gorhom plans to porting it to v3?

@chybisov
Copy link

@gorhom could you please suggest the way how to achieve same functionality on v3? This is critical feature, because now it's not clear how to address these issues:

  1. Integrate with something that needs to be below bottom sheet (e.g. bottom navigation bar).
  2. On iPhone X/11/12 we have bottom line and bottom sheet should take into account its height.

I'm not sure where this should be done - on lib side or our own, but tips to this would be so much helpful. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Footer Component
5 participants