SteffeyDev / react-native-popover-view

A well-tested, adaptable, lightweight <Popover> component for react-native
MIT License
614 stars 92 forks source link

Subtract verticalOffset from popover height to preserve margins on popover #49

Closed outaTiME closed 3 years ago

outaTiME commented 5 years ago

Hi, verticalOffset still required on Android on version 2.x when running on expo, and may update the popover height according the vertical offset too (for better experience on large popovers like the image attached)

here snippet and screenshots, thanks and awesome work !!!

btw: on iOS works as expected.

<TouchableOpacity
  ref={ref => (this._controls_button_ref = ref)}
  style={{
    position: 'absolute',
    top: 0,
    right: 0
  }}
  onPress={this._showControls}
>
  <Text>Button</Text>
</TouchableOpacity>
<Popover
  isVisible={showControls}
  fromView={this._controls_button_ref}
  onRequestClose={this._onPressCancel}
  verticalOffset={
    Platform.OS === 'android' ? -Constants.statusBarHeight : 0
  }
>
  <View>Stuff here</View>
</Popover>

telegram-cloud-photo-size-1-5154761118516881495-y

SteffeyDev commented 5 years ago

I’m not entirely sure what the issue is, the popover in the image appears to be working as expected, except maybe slightly offset horizontally. Can you please restate what the issue is exactly, describing the expected behavior and actual behavior of the popover?

outaTiME commented 5 years ago

Hi @SteffeyDev, the popover height should sustract verticalOffset for better vertical alignment on the screen.

And additionally I made the clarification that the verticaloffset is still required in Expo.

SteffeyDev commented 5 years ago

I agree that the verticalOffset should shift the popover, and from your picture it appears to be working correctly, because if it were not the arrow would not point to the button. Again, what is the issue? What is not working correctly?

outaTiME commented 5 years ago

Yup, its working correctly but this is a little enhacement for better popview alignment, check out the following image:

64390847-d3a55400-d01c-11e9-94c4-6664412159fc

I hope it serves as clarification 🙏

SteffeyDev commented 5 years ago

Ah that is amazing clarification, thanks! I'll do that as part of the next release.

SteffeyDev commented 3 years ago

In the latest version (4.0.0), the vertical offset is applied before margins, so this should be fixed. Re-open if it is still an issue in the latest version.