search

SteffeyDev / react-native-popover-view

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

bug when selecting full screen text #32

Closed hweister closed 4 years ago

hweister commented 5 years ago

in this case, no screen place to display the popover view, the forcedContentSize.height < 0

SteffeyDev commented 5 years ago

Sorry for not seeing this sooner, can you attach a screenshot or video of what you are seeing? I’m not understanding the bug

hweister commented 5 years ago

Sorry I don't have case on hand to reproduce it now. I will try to describe the problem clear: In my app, when I long press a view, the popover will show: <Text onLongPress={() => { onLongPress && this.setState({showPopoverView: true}); }} ref={ref => this.touchable = ref} >... <Popover isVisible={showPopoverView} placement={'auto'} fromView={this.touchable} .../> Then, the pop over view will try to find a place with 'auto' mode, with this sequence: left --> right --> top --> bottom. as regard to 'top' and 'bottom' mode, it will compare the margin, to find a better place to show. If the 'fromView' is full of screen, which means it can not find a place to display the content, by default, a modal full of screen will show, but no content displayed. I forked your code and made two update for this case:

  1. https://github.com/hweister/react-native-popover-view/commit/3d50ca3ccf897f732cb070fe89e7e2b221d75e91 add code in line 802 of src/Popover.js file if (forcedContentSize && ( forcedContentSize.height < 0 || forcedContentSize.width < 0)) { return null; }

  2. https://github.com/hweister/react-native-popover-view/commit/2d8b390b6e183113950e62238c92a0bb3f6f26a3 support to show the pop over view in the center of the 'fromView'

SteffeyDev commented 5 years ago

Ok, I'm thinking of a fix for this that is more consistent with the rest of my code, I'll make the change and push and update and we can see if it works for you

SteffeyDev commented 5 years ago

How does this sound: If the popover can't fit on any side of the view, then just show it as centered on screen as if there were no view.

Would that work?

hweister commented 5 years ago

I prefer try to show on the center of the selected view first, if it still can not find enough place, then show on the center of the screen. Thanks

SteffeyDev commented 5 years ago

That's a great idea, I'll add that in and get it tested in various scenarios

SteffeyDev commented 5 years ago

Ok, I'm curious if the new version 1.0.17 fixes your issue, there's a lot of edge cases when it comes to this and not sure if I got enough of them. Check it out when you get the chance.

sghosh968 commented 5 years ago

@SteffeyDev : I am not sure if the issue I am facing is related to issue reported here. Issue: Popover modal when triggered to open in a longPress handler moves up & down multiple times. Issue is on iOS only when the popover is triggered in a onLongPress handler for TouchableOpacity. But works fine when triggered in onPress for TouchableOpacity. Added a video below demonstrating the issue on simulator. Other details: react-native-popover-view : 2.0.1 react-native : 0.58.3 Link to code : https://github.com/sghosh968/PopoverTest Link to video: https://drive.google.com/file/d/16gHQp0xgTfwdE8AfmUuvRnIRYaBUyT8S/view

SteffeyDev commented 5 years ago

I don't think this is related, could you open a new issue? I have a basic idea of what causes this, but it is very weird that it would occur only with long press. I'll test it out when I have a second and see what's up.

SteffeyDev commented 4 years ago

Closing due to inactivity (and probably fixed)