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

Opening any popover scrolls to bottom of page in Firefox #231

Closed
adidahiya opened this issue Nov 23, 2016 · 10 comments
Closed

Opening any popover scrolls to bottom of page in Firefox #231

adidahiya opened this issue Nov 23, 2016 · 10 comments

Comments

@adidahiya
Copy link
Contributor

Bug Report

  • Package version(s): 1.0.1 (blueprintjs.com)
  • Browser and OS versions: FF 46 & 50, macOS 10.12

Describe the issue:

Clicking on any popover scrolls to the bottom of its overlay, which is now the bottom of the page (related: #186 (comment)).

@giladgray
Copy link
Contributor

giladgray commented Nov 25, 2016

this is due to position: absolute nature of overlays.
i think we should make them all position: fixed as in #183 (comment).

@giladgray giladgray added this to the 1.2.0 milestone Nov 29, 2016
@giladgray
Copy link
Contributor

related to #183

@cmslewis
Copy link
Contributor

cmslewis commented Dec 1, 2016

Okay, so building off of my work in #276, I'm now seeing the following behavior:

  • Chrome: All popovers work as expected
  • Firefox: All popovers misbehave except for the "Input" example.
  • Safari: All popovers work except for the "Input" example.

So that's interesting. Lest I forget, the offending line seems to be in Overlay/bringFocusedInsideOverlay (linked here):

containerElement.focus()

This is triggered by the Portal's onChildrenMount callback.

Okay, diving back in.

@cmslewis
Copy link
Contributor

cmslewis commented Dec 1, 2016

In Firefox, looks like we're trying to .focus on the popover container before it exists in the DOM (despite checks against that). Since the popover's portal wrapper exists at the bottom of the page, the browser is scrolling down to bring the portal into view before trying to focus it. Adding a setTimeout around the containerElem.focus() solves that issue.

But Safari appears to have a different problem: it's scrolling the page when Portal invokes ReactDOM.unstable_renderSubtreeIntoContainer.

Diving back in.

@adidahiya
Copy link
Contributor Author

By the way, I'm also seeing this issue with the Overlay example in Firefox.

@cmslewis
Copy link
Contributor

cmslewis commented Dec 5, 2016

Been investigating this for awhile. Two interim takeaways:

  1. Setting .pt-portal to position: fixed fixes the scrolling-to-the-bottom-of-the-page issue.
  2. BUT setting .pt-portal to position: fixed introduces a new bug where tethered content is moved outside of its React parent due to this snippet of code in tether.js:
    image

This leads to badness wherein the portal content can't be properly updated or removed from the DOM. I'm trying a workaround where we hot-swap the position setting quickly (e.g. set to static for the render and tether updating, then immediately reset to fixed). But it's not working great at the moment.

More to come, I hope.

@adidahiya
Copy link
Contributor Author

I'm trying a workaround where we hot-swap the position setting quickly (e.g. set to static for the render and tether updating, then immediately reset to fixed

this sounds horrific, FWIW 😆

@cmslewis
Copy link
Contributor

cmslewis commented Dec 5, 2016

This whole thing is horrific. -.-

@giladgray
Copy link
Contributor

@cmslewis yeah tether will do that moving thing sometimes...
this may be useful: http://tether.io/#element-moving

@michael-yx-wu
Copy link
Contributor

This also happens with context menus. For example, try right clicking on the table sortable example.

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

No branches or pull requests

5 participants