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

Issue with scrolling using the flot library #162

Closed
anniesullie opened this issue May 24, 2013 · 12 comments
Closed

Issue with scrolling using the flot library #162

anniesullie opened this issue May 24, 2013 · 12 comments
Assignees

Comments

@anniesullie
Copy link

I made a page that uses flot to plot some charts inside custom elements. When I scroll down the page, the flot library plothover event sends events with incorrect data. It works correctly if I use plain HTML instead of a custom element.

Here is a test case I made:
https://github.com/anniesullie/polymer-flot-bug

The file flot-scroll.html demonstrates the bug, and the file flot-scroll-no-polymer.html demonstrates that there is no bug when I use plain html without polymer.

@dfreedm
Copy link
Member

dfreedm commented May 24, 2013

There's something funky going on here with how the chart finds it's offset to generate the x/y in the plothover. That the bug also manifests in shadowdom polyfill tells me that some sort of tree walking is missing from shadowroot -> host.

@ghost ghost assigned dfreedm Jun 12, 2013
@anniesullie
Copy link
Author

Will this be addressed in Polymer at all? It's really difficult to use third party libraries with Polymer due to this issue. I know there are better ways the third-party code could get the position, but I don't have control over the code.

Note that even though it's not ideal this is pretty common practice; all the top results for queries like "element position javascript" recommend this approach. Example: http://www.quirksmode.org/js/findpos.html

@dfreedm
Copy link
Member

dfreedm commented Jun 20, 2013

I need to look through this again, and see if I can make a smaller repro case. Flot has a bunch of crazy minified stuff that makes it hard to follow the callstack.

To answer your element position question, the good way is to use getBoundingClientRect(), which doesn't have to walk the parents.

I suspect we could hack in offsetParent by using the host's offset parent.

I'll make sure both a) I'm correct about getBoundingClientRect, and b) offsetParent hack would work.

@arv
Copy link
Contributor

arv commented Jun 21, 2013

Maybe offsetParent should work across the shadow boundary?
On Jun 20, 2013 6:48 PM, "Daniel Freedman" notifications@github.com wrote:

I need to look through this again, and see if I can make a smaller repro
case. Flot has a bunch of crazy minified stuff that makes it hard to follow
the callstack.

To answer your element position question, the good way is to use
getBoundingClientRect(), which doesn't have to walk the parents.

I suspect we could hack in offsetParent by using the host's offset parent.

I'll make sure both a) I'm correct about getBoundingClientRect, and b)
offsetParent hack would work.


Reply to this email directly or view it on GitHubhttps://github.com//issues/162#issuecomment-19788538
.

@dfreedm
Copy link
Member

dfreedm commented Jun 21, 2013

Oh no, I was wrong, flot is using getBoundingClientRect
However, it depends on document.contains to be true in order to add window.pageYOffset to the output top.

This is something I've actually filed a spec bug about for ShadowDOM: https://www.w3.org/Bugs/Public/show_bug.cgi?id=22141

@dfreedm
Copy link
Member

dfreedm commented Jun 21, 2013

@dglazkov, for spec use case.

@anniesullie , if you add window.pageYOffset to the plothover y axis, it should work I think.

@dfreedm
Copy link
Member

dfreedm commented Jul 24, 2013

@anniesullie is this still blocking you?

@anniesullie
Copy link
Author

Sorry I didn't reply earlier! We patched document.contains to return true if the element is in the shadow dom for one of our graphs and everything is working great. Thanks so much for your help!

@dfreedm
Copy link
Member

dfreedm commented Jul 24, 2013

Cool. @dglazkov was convinced in the spec bug that document.contains should work for shadowed elements, so hopefully your shim will be unnecessary soon.

@dfreedm dfreedm closed this as completed Jul 24, 2013
@czellweg
Copy link

@anniesullie I'm facing almost the same problem. Would you mind sharing your solution, i.e. how you patched document.contains? Your help would be greatly appreciated.

@anniesullie
Copy link
Author

@czellweg - The code that called into document.contains was actually jQuery, so we ended up patching the jQuery implementation of document.contains. Here's the code, which is quite hacky--you could do something more general case for the 'MY ELEMENT HERE' part, of course.

// Workaround for document.contains returning false for elements in the
// shadow DOM. jQuery mouse events need it to return true for scrolling
// to be properly accounted for. For background, see
// https://github.com/Polymer/polymer/issues/162 and
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=22141
jQuery.contains = function(doc, elem) {
  shadowElems = document.getElementsByTagName('ELEMENT NAME HERE');
  for (var i = 0; i < shadowElems.length; i++) {
    if (shadowElems[i].shadowRoot.contains(elem)) {
      return true;
    }
  }
  return doc.contains(elem);
}

@azakus @dglazkov - Did the spec and implementation ever get updated to deal with this better?

@czellweg
Copy link

@anniesullie Awesome, thanks for your quick reply - worked like a charm!

Have you experienced any issues so far with this approach? I will use it for now and keep an eye out for the 'proper' fix / way to do it.

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

4 participants