search

SteffeyDev / react-native-popover-view

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

When click on button for multiple times popover is opening and screen got strucked? #37

Open Anshu8405 opened 5 years ago

Anshu8405 commented 5 years ago

Here is my sample code you can look into it.

showPopover() {
   this.setState({isVisible: true});
 }

 closePopover() {
    this.setState({isVisible: false});
 }

 render() {
    return (

   <View>        
    <TouchableHighlight ref={ref => this.touchable = ref} style={styles.button} onPress={() => this.showPopover()}> <Text>Press me</Text> </TouchableHighlight> 
    <Popover
      isVisible={this.state.isVisible}
      fromView={this.touchable}
      onClose={() => this.closePopover()}>
      <Text>I'm the content of this popover!</Text>
    </Popover>
  </view>

 );
}
Anshu8405 commented 5 years ago

Here is my sample code you can look into it.

showPopover() {
   this.setState({isVisible: true});
 }

 closePopover() {
    this.setState({isVisible: false});
 }

 render() {
    return (

   <View>        
    <TouchableHighlight ref={ref => this.touchable = ref} style={styles.button} onPress={() => this.showPopover()}> <Text>Press me</Text> </TouchableHighlight> 
    <Popover
      isVisible={this.state.isVisible}
      fromView={this.touchable}
      onClose={() => this.closePopover()}>
      <Text>I'm the content of this popover!</Text>
    </Popover>
  </view>

 );
}
SteffeyDev commented 5 years ago

So you are just spamming the touchable? What version of react-native and react-native-popover-view are you using? I can test this out when I have a free second.

adp-source commented 4 years ago

We're also having this problem. If you spam the touchable, eventually the popover will just disappear. There's no way to get it back unless you reload.

Using onOpenComplete or onCloseComplete to disable the touchable doesn't work because javascript can't disable native animations.

It is likely that the user can press something multiple times quickly.

SteffeyDev commented 4 years ago

Is this with tooltip mode or regular?

adp-source commented 4 years ago

@SteffeyDev Regular mode. Although I guess we're using it as a tooltip to auto close. I think an option to disable animations would be useful.

SteffeyDev commented 4 years ago

You can disable the animations by using the animationConfig prop and passing a duration of 0

ergenekonyigit commented 3 years ago

We're having same problem, I tried to disable animation but problem not fixed.

rn version: 63.2 popover-view version: 4.0.1

SteffeyDev commented 3 years ago

@ergenekonyigit can you provide reproduction steps? Can you reproduce consistently?

AdamGerthel commented 2 years ago

This is happening to me too. It's not super easy to reproduce, but it seems to happen more easily if you stop pressing the button while the animation is ongoing. There's no error and the screen becomes unresponsive. I'm on 5.1.2 but it happened with 4.x as well.

Update: I recorded it -> https://user-images.githubusercontent.com/1490370/188266932-dd42a45e-cf19-48e3-9a23-c233d4175d41.mp4

Note that I tried for ~20 seconds before as well (trimmed the video).

Update 2: I tried with animation duration set to 0 and was still able to reproduce it.

SteffeyDev commented 2 years ago

I'm unable to play that video, can you try re-upload or upload on a different platform? Also, can you include debug logs?

AdamGerthel commented 2 years ago

Here's a new video: https://user-images.githubusercontent.com/1490370/189649072-91c42527-bf73-4880-8b5f-7ec99ff53b7c.mp4

I'm using longpress to display the popover. Mode is PopoverMode.RN_MODAL. I'm actually thinking that the problem could be that it doesn't close properly rather than freezes, but that there's no way to get rid of it once it's opened and the finger is off.

Logs:

 LOG  [2022-09-12T12:02:11.809Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:11.815Z] calculateRectFromRef - waiting for ref to move from: {"x":0,"y":0,"width":0,"height":0}
 LOG  [2022-09-12T12:02:11.923Z] calculateRectFromRef - calculated Rect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:11.963Z] setDefaultDisplayArea - newDisplayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:11.963Z] setDefaultDisplayArea - displayAreaOffset: {"x":0,"y":0}
 LOG  [2022-09-12T12:02:11.967Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:11.968Z] calculateRectFromRef - waiting for ref to move from: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:11.989Z] measureContent - new requestedContentSize: {"width":276,"height":113.5} (used to be null)
 LOG  [2022-09-12T12:02:11.993Z] handleChange - waiting 100ms to accumulate all changes
 LOG  [2022-09-12T12:02:12.106Z] handleChange - requestedContentSize: {"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:12.107Z] handleChange - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:12.107Z] handleChange - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:12.107Z] handleChange - placement: "auto"
 LOG  [2022-09-12T12:02:12.111Z] computeAutoGeometry - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:12.112Z] computeAutoGeometry - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:12.112Z] computeAutoGeometry - List of available space: {"left":{"sizeAvailable":94,"sizeRequested":276,"fits":false,"extraSpace":-182},"right":{"sizeAvailable":208,"sizeRequested":276,"fits":false,"extraSpace":-68},"top":{"sizeAvailable":692.5,"sizeRequested":113.5,"fits":true,"extraSpace":579},"bottom":{"sizeAvailable":91.5,"sizeRequested":113.5,"fits":false,"extraSpace":-22}}
 LOG  [2022-09-12T12:02:12.112Z] computeAutoGeometry - Found best postition for placement: "top"
 LOG  [2022-09-12T12:02:12.114Z] computeGeometry - initial chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:12.114Z] computeGeometry - final chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:12.118Z] handleChange - animating in
 LOG  [2022-09-12T12:02:12.119Z] getTranslateOrigin - popoverSize: {"width":276,"height":121.5}
 LOG  [2022-09-12T12:02:12.119Z] getTranslateOrigin - anchorPoint: {"x":150,"y":672.5}
 LOG  [2022-09-12T12:02:12.119Z] animateIn - translateStart: {"x":12,"y":2403.75}
 LOG  [2022-09-12T12:02:12.119Z] animateIn - translatePoint: {"x":12,"y":551}
 LOG  [2022-09-12T12:02:12.124Z] Setting up keyboard listeners
 LOG  [2022-09-12T12:02:12.192Z] [BasePopover] componentDidUpdate - changedProps: ["isVisible"]
 LOG  [2022-09-12T12:02:12.192Z] componentDidUpdate - isVisible changed, now false
 LOG  [2022-09-12T12:02:12.192Z] componentDidUpdate - Hiding popover
 LOG  [2022-09-12T12:02:12.431Z] animateOut - isMounted: true
 LOG  [2022-09-12T12:02:12.432Z] getTranslateOrigin - popoverSize: {"width":276,"height":121.5}
 LOG  [2022-09-12T12:02:12.432Z] getTranslateOrigin - anchorPoint: {"x":150,"y":672.5}
 LOG  [2022-09-12T12:02:12.435Z] Tearing down keyboard listeners
 LOG  [2022-09-12T12:02:12.444Z] animateIn - onOpenComplete - Calculated Popover Rect: {"x":12,"y":551,"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:12.445Z] animateIn - onOpenComplete - Calculated Arrow Rect: {"x":142,"y":662.5,"width":18,"height":10}
 LOG  [2022-09-12T12:02:13.666Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:13.667Z] calculateRectFromRef - waiting for ref to move from: {"x":0,"y":0,"width":0,"height":0}
 LOG  [2022-09-12T12:02:13.789Z] calculateRectFromRef - calculated Rect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:13.838Z] setDefaultDisplayArea - newDisplayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:13.838Z] setDefaultDisplayArea - displayAreaOffset: {"x":0,"y":0}
 LOG  [2022-09-12T12:02:13.844Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:13.844Z] calculateRectFromRef - waiting for ref to move from: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:13.855Z] measureContent - new requestedContentSize: {"width":276,"height":113.5} (used to be null)
 LOG  [2022-09-12T12:02:13.861Z] handleChange - waiting 100ms to accumulate all changes
 LOG  [2022-09-12T12:02:13.910Z] [BasePopover] componentDidUpdate - changedProps: ["isVisible"]
 LOG  [2022-09-12T12:02:13.910Z] componentDidUpdate - isVisible changed, now false
 LOG  [2022-09-12T12:02:13.910Z] componentDidUpdate - Hiding popover
 LOG  [2022-09-12T12:02:13.972Z] handleChange - requestedContentSize: {"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:13.972Z] handleChange - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:13.973Z] handleChange - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:13.973Z] handleChange - placement: "auto"
 LOG  [2022-09-12T12:02:13.973Z] computeAutoGeometry - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:13.973Z] computeAutoGeometry - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:13.973Z] computeAutoGeometry - List of available space: {"left":{"sizeAvailable":94,"sizeRequested":276,"fits":false,"extraSpace":-182},"right":{"sizeAvailable":208,"sizeRequested":276,"fits":false,"extraSpace":-68},"top":{"sizeAvailable":692.5,"sizeRequested":113.5,"fits":true,"extraSpace":579},"bottom":{"sizeAvailable":91.5,"sizeRequested":113.5,"fits":false,"extraSpace":-22}}
 LOG  [2022-09-12T12:02:13.974Z] computeAutoGeometry - Found best postition for placement: "top"
 LOG  [2022-09-12T12:02:13.974Z] computeGeometry - initial chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:13.974Z] computeGeometry - final chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:13.980Z] handleChange - animating in
 LOG  [2022-09-12T12:02:13.981Z] getTranslateOrigin - popoverSize: {"width":276,"height":121.5}
 LOG  [2022-09-12T12:02:13.981Z] getTranslateOrigin - anchorPoint: {"x":150,"y":672.5}
 LOG  [2022-09-12T12:02:13.981Z] animateIn - translateStart: {"x":12,"y":2403.75}
 LOG  [2022-09-12T12:02:13.982Z] animateIn - translatePoint: {"x":12,"y":551}
 LOG  [2022-09-12T12:02:13.986Z] Setting up keyboard listeners
 LOG  [2022-09-12T12:02:14.279Z] animateOut - isMounted: true
 LOG  [2022-09-12T12:02:14.279Z] getTranslateOrigin - popoverSize: {"width":276,"height":121.5}
 LOG  [2022-09-12T12:02:14.280Z] getTranslateOrigin - anchorPoint: {"x":150,"y":672.5}
 LOG  [2022-09-12T12:02:14.282Z] Tearing down keyboard listeners
 LOG  [2022-09-12T12:02:14.293Z] animateIn - onOpenComplete - Calculated Popover Rect: {"x":12,"y":551,"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:14.293Z] animateIn - onOpenComplete - Calculated Arrow Rect: {"x":142,"y":662.5,"width":18,"height":10}
 LOG  [2022-09-12T12:02:15.616Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:15.617Z] calculateRectFromRef - waiting for ref to move from: {"x":0,"y":0,"width":0,"height":0}
 LOG  [2022-09-12T12:02:15.725Z] calculateRectFromRef - calculated Rect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:15.763Z] setDefaultDisplayArea - newDisplayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:15.763Z] setDefaultDisplayArea - displayAreaOffset: {"x":0,"y":0}
 LOG  [2022-09-12T12:02:15.768Z] calculateRectFromRef - waiting for ref
 LOG  [2022-09-12T12:02:15.768Z] calculateRectFromRef - waiting for ref to move from: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:15.789Z] measureContent - new requestedContentSize: {"width":276,"height":113.5} (used to be null)
 LOG  [2022-09-12T12:02:15.796Z] handleChange - waiting 100ms to accumulate all changes
 LOG  [2022-09-12T12:02:15.906Z] handleChange - requestedContentSize: {"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:15.906Z] handleChange - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:15.906Z] handleChange - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:15.907Z] handleChange - placement: "auto"
 LOG  [2022-09-12T12:02:15.907Z] computeAutoGeometry - displayArea: {"x":0,"y":0,"width":414,"height":896}
 LOG  [2022-09-12T12:02:15.907Z] computeAutoGeometry - fromRect: {"x":110,"y":708.5,"width":80,"height":80}
 LOG  [2022-09-12T12:02:15.907Z] computeAutoGeometry - List of available space: {"left":{"sizeAvailable":94,"sizeRequested":276,"fits":false,"extraSpace":-182},"right":{"sizeAvailable":208,"sizeRequested":276,"fits":false,"extraSpace":-68},"top":{"sizeAvailable":692.5,"sizeRequested":113.5,"fits":true,"extraSpace":579},"bottom":{"sizeAvailable":91.5,"sizeRequested":113.5,"fits":false,"extraSpace":-22}}
 LOG  [2022-09-12T12:02:15.908Z] computeAutoGeometry - Found best postition for placement: "top"
 LOG  [2022-09-12T12:02:15.908Z] computeGeometry - initial chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:15.908Z] computeGeometry - final chosen geometry: {"popoverOrigin":{"x":12,"y":551},"anchorPoint":{"x":150,"y":672.5},"placement":"top","forcedContentSize":{"width":394,"height":654.5},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-09-12T12:02:15.916Z] handleChange - animating in
 LOG  [2022-09-12T12:02:15.917Z] getTranslateOrigin - popoverSize: {"width":276,"height":121.5}
 LOG  [2022-09-12T12:02:15.917Z] getTranslateOrigin - anchorPoint: {"x":150,"y":672.5}
 LOG  [2022-09-12T12:02:15.918Z] animateIn - translateStart: {"x":12,"y":2403.75}
 LOG  [2022-09-12T12:02:15.918Z] animateIn - translatePoint: {"x":12,"y":551}
 LOG  [2022-09-12T12:02:15.924Z] Setting up keyboard listeners
 LOG  [2022-09-12T12:02:16.220Z] animateIn - onOpenComplete - Calculated Popover Rect: {"x":12,"y":551,"width":276,"height":113.5}
 LOG  [2022-09-12T12:02:16.222Z] animateIn - onOpenComplete - Calculated Arrow Rect: {"x":142,"y":662.5,"width":18,"height":10}
dantxal commented 2 years ago

Hello! I work with @AdamGerthel and I created a snack where you will be able to reproduce the error:

EDIT: I made the implementation simpler, basically what we want to achieve is: Have a button inside the Popover component that can be disabled but still show the popover, and have the popover hide with onPressOut event.

You can reproduce the error on FIRST TRY if you use the web view. I believe that is because the onPressOut event happens right after the animation starts.

Problem: Using onRequestClose from the Popover component made it update the state, but doesn't update the component's render.

https://snack.expo.dev/@dantxal/custom-popover

dantxal commented 2 years ago

I found a hacky way to solve it.

What seems to happen is, since the onPressOut event happens in the middle of the animation, the showPopover state variable gets set to false in the middle of the animation. So the animation ends (showing the popover), now the popover is shown even if the showPopover variable is false.

The HACKY fix

I added onOpenComplete={show} which will update the popover state variable to true again once the animation ends.

Then, if we press anywhere on the screen again, it hides the popover.

We get this warning though:

Popover Warning - Can't Show - Attempted to show a Popover while another one was already showing.  
You can only show one Popover at a time, and must wait for one to close completely before showing a different one.  
You can use the onCloseComplete prop to detect when a Popover has finished closing.  
To show multiple Popovers simultaneously, all but one should have mode={Popover.MODE.JS_MODAL}.  
Once you change the mode, you can show as many Popovers as you want, but you are responsible for keeping them above other views.

Is there a proper way to solve this desynchronization between Popover visibility and the state variable?

SteffeyDev commented 2 years ago

So, this should be handled internally already... If isVisible is set to false while it is animating in, it sets the animateOutAfterShow flag to true which automatically triggers the animate out once the animate in completes. It looks like that mechanism is working most of the time, and it's only in a race condition that it fails. From the video, it looks like you do an additional click while the popover is animating in, and that causes it to get stuck. I'll hopefully have more time to investigate further this week.

AdamGerthel commented 2 years ago

From the video, it looks like you do an additional click while the popover is animating in, and that causes it to get stuck.

That's correct. This can happen from button-mashing in a game (as in our case), but it could also be someone with trembling hands (i.e. Parkinson disease etc.).

umangcodealchemy commented 1 year ago

Hii, try this

animationConfig={{ duration: 500, easing: Easing.inOut(Easing.quad) }}

AdamGerthel commented 1 month ago

So, this should be handled internally already... If isVisible is set to false while it is animating in, it sets the animateOutAfterShow flag to true which automatically triggers the animate out once the animate in completes.

After some initial investigation, I've found that this never happens, because componentDidUpdate is not triggered in time. I'm not sure why, but I'm starting to think that the various setTimeout related to onDisplayAreaChanged is causing the problems. I suspect that they're interfering with the "natural" rendering sequence, but I'm not sure.

Here's a video where I've recorded it happening in the test app / demo. As you can see from the code sample, I'm using longPress with onPressOut to handle opening and closing the popover. I've added a couple of additional console logs in order to get a better understanding of the sequence of things:

https://github.com/user-attachments/assets/9fae82b0-d7c5-4fb4-bf2f-a763f5ef34b7

It looks like BasePopover never has time to receive the isVisible: true prop, so when componentDidUpdate is called, both the new and previous prop will be false, meaning that this line is never reached. I'm not quite sure how that can be, yet.

AdamGerthel commented 1 month ago

@SteffeyDev do you remember why the timeouts in calculateRectFromRef exist, and maybe give some more context as to what that function does? I suspect that the timeouts there could be the reason for #177 as well, could that be true?

SteffeyDev commented 1 month ago

The function as a whole just gets the rectangle bounds for a given component, specified by a ref. It looks like the first timeout is just a sleep function that waits for the ref to be set, since refs are initially null and are set during the render cycle sometime. The second one seems to be waiting for the rectangle to change, which is needed both in the initialization (waiting for us to get the first rect) or to react to a display area change (waiting to get the new rect).