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

Reposition a popover while it is open #1438

Closed
thetimbanks opened this issue Oct 22, 2015 · 4 comments
Closed

Reposition a popover while it is open #1438

thetimbanks opened this issue Oct 22, 2015 · 4 comments

Comments

@thetimbanks
Copy link
Contributor

I am running in to a case where I am updating content inside a popover dynamically and the position of the popover ends up in the wrong place. I have put together a fiddle showing what I am experiencing.

https://jsfiddle.net/thetimbanks/L8yc6a3p/1/

I have dug through the code a little bit and it looks like the positioning of the popover is handled in the updatePosition method of the Position component. There are some checks to make sure to not try to reposition if the placement hasn't changed. In my case the placement hasn't changed, but the content has and therefor would need to be moved. Here is a proposed solution to the updatePosition method:

updatePosition() {
    const target = this.getTargetSafe();

    if (!target) {
      this.setState({
        positionLeft: null,
        positionTop: null,
        arrowOffsetLeft: null,
        arrowOffsetTop: null
      });

      return;
    }    

    const overlay = ReactDOM.findDOMNode(this);
    const container = getContainer(this.props.container, ownerDocument(this).body);

    this.setState(calcOverlayPosition(
      this.props.placement,
      overlay,
      target,
      container,
      this.props.containerPadding
    ));
}

This gets rid of the placementChanged check and always updates the position as long as there is a target. Is the cost of calculating the position and setting a new position that costly?

I would be happy to throw together a PR for this if you would prefer to see it in action.

@taion
Copy link
Member

taion commented Oct 22, 2015

This has to be opt-in - the issue is that otherwise for whatever reason we end up with minor jitter in the popover positioning even when the popover itself is static.

@thetimbanks
Copy link
Contributor Author

@taion That makes sense. Would you see the setting as being a prop on the Overlay/OverlayTrigger components and then passed down through to the Position component similar to how the placement prop is handled?

@taion
Copy link
Member

taion commented Oct 23, 2015

Exactly so.

@taion
Copy link
Member

taion commented Dec 24, 2015

Closed by react-bootstrap/react-overlays#42.

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

No branches or pull requests

2 participants