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

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

Open
Anshu8405 opened this issue Mar 22, 2019 · 19 comments
Labels

Comments

@Anshu8405
Copy link

Anshu8405 commented Mar 22, 2019

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
Copy link
Author

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 Anshu8405 reopened this Mar 22, 2019
@SteffeyDev
Copy link
Owner

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
Copy link

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
Copy link
Owner

Is this with tooltip mode or regular?

@adp-source
Copy link

@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
Copy link
Owner

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

@ergenekonyigit
Copy link

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
Copy link
Owner

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

@AdamGerthel
Copy link
Collaborator

AdamGerthel commented Sep 3, 2022

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
Copy link
Owner

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
Copy link
Collaborator

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
Copy link

dantxal commented Sep 17, 2022

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
Copy link

dantxal commented Sep 17, 2022

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
Copy link
Owner

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
Copy link
Collaborator

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
Copy link

Hii, try this

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

@AdamGerthel
Copy link
Collaborator

AdamGerthel commented Oct 4, 2024

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:

Screen.Recording.2024-10-04.at.10.30.16.mp4

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
Copy link
Collaborator

@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
Copy link
Owner

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).

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

No branches or pull requests

7 participants