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

Popover causes the browser to crash if window size is changed #1488

Closed
liemdo opened this issue May 10, 2019 · 10 comments · Fixed by #1669
Closed

Popover causes the browser to crash if window size is changed #1488

liemdo opened this issue May 10, 2019 · 10 comments · Fixed by #1669

Comments

@liemdo
Copy link

liemdo commented May 10, 2019

  • components: popover
  • reactstrap version 8.0.0

Steps to reproduce the issue

  1. Open the popover in Chrome or Safari (I didn't test with other browsers)
  2. Change the window size
  3. The browser crashes. I have to force quit the browser.
@TheSharpieOne
Copy link
Member

I cannot reproduce. Does this happen for you on https://reactstrap.github.io/components/popovers/ ?

@liemdo
Copy link
Author

liemdo commented May 10, 2019

You have to resize the window quickly so that part of the popover is out of the window view. The browser crashes right after that.

reactstrap

If you open the Chrome Devtools while the popover is open, you'll get the same issue.

This issue may be related to this one #1482

@morel333
Copy link

I cannot reproduce. Does this happen for you on https://reactstrap.github.io/components/popovers/ ?

Open your samples in minimal browser size and click on any popover - it will reproduce.
Looks like ther's an infinite state placement change in PopperContent in handlePlacementChange (placement changes top/bottom -> etc).

@vitorblz
Copy link

The same happens with tooltips

@morel333
Copy link

As a hotfix changed
<ReactPopper referenceElement={this.targetNode} modifiers={extendedModifiers} placement={placement} >
to
<ReactPopper referenceElement={this.targetNode} modifiers={extendedModifiers} placement={attrs.placement} >
in PopperContent

@usman-subhani
Copy link

usman-subhani commented May 31, 2019

Tooltiperror

My browser crashes as well while using tooltips.Its a bit tricky to reproduce but as far as i know it often happens when mouse moves to quickly over the tooltip hover element or mouse hovers over a tooltip element during/after scrolling. I am using uncontrolled tooltips with delays mentioned below.
I am using v8.0.0 of reactstrap.

const tooltipDelay = { show: 500, hide: 300 };

const UncontrolledTooltipWithDelay = props => (
  <UncontrolledTooltip delay={tooltipDelay} trigger="hover" {...props} />
);

Edit: Nevermind, this is related to #1482

@Phibedy
Copy link

Phibedy commented Jun 1, 2019

For me it happens if the tooltip does not fit in the window. As long as the tooltip fits on the page it works fine but if I keep decreasing the window size it fails.
image

Edit: Nevermind, this is related to #1482

@YassienW
Copy link

YassienW commented Jun 4, 2019

@usman-subhani Would this fix your issue? #1482 (comment)

@usman-subhani
Copy link

usman-subhani commented Jun 6, 2019

@usman-subhani Would this fix your issue? #1482 (comment)

@YassienW Yes downgrading to v7.1.0 of reactstrap resolved this for me. Thanks 👍

Also setting the tooltip prop flip to false also resolves the issue in v8.0.0

@jesusgp22
Copy link

This issue also happens if the popover is bigger than the current viewport on opening the popover

phwebi added a commit to phwebi/reactstrap that referenced this issue Oct 17, 2019
…ining element

When a tooltip or popover does not fit in the containing element, this causes the `PopperContent` component to get into an infinite loop of switching between `placement`s in its state. This is because we attempt to update `this.state.placement` everytime popper.js flips the placement to find a better fit and pass it back as the `placement` property to `ReactPopper`.

This is a problem because the `PopperContent` component has two sources of truth for its placement. One coming from its own `placement` property, and one coming from the internal data of `ReactPopper`.

This commit fixes the issue by using `PopperContent`'s `placement` property as the initial placement of the `ReactPopper` component and allowing `ReactPopper` to manage its own updates (i.e. if the popover needs to be flipped because there's not enough room for the specified placement.)

Fixes reactstrap#1482
Fixes reactstrap#1488
@phwebi phwebi mentioned this issue Oct 17, 2019
12 tasks
TheSharpieOne pushed a commit that referenced this issue Oct 18, 2019
When a tooltip or popover does not fit in the containing element, this causes the `PopperContent` component to get into an infinite loop of switching between `placement`s in its state. This is because we attempt to update `this.state.placement` everytime popper.js flips the placement to find a better fit and pass it back as the `placement` property to `ReactPopper`.

This is a problem because the `PopperContent` component has two sources of truth for its placement. One coming from its own `placement` property, and one coming from the internal data of `ReactPopper`.

This commit fixes the issue by using `PopperContent`'s `placement` property as the initial placement of the `ReactPopper` component and allowing `ReactPopper` to manage its own updates (i.e. if the popover needs to be flipped because there's not enough room for the specified placement.)

Fixes #1482
Fixes #1488
Fixes #1664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants