Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(tooltip): always append to body and disappear on scroll #6140

Closed
wants to merge 1 commit into from

Conversation

EladBezalel
Copy link
Member

  • Default animation origin has been set to bottom that mean the origin is top center

@jelbourn please review

- Default animation origin has been set to bottom that mean the origin is `top center`
@EladBezalel EladBezalel added the needs: review This PR is waiting on review from the team label Dec 8, 2015
ngWindow.on('blur', windowBlurHandler);
ngWindow.on('resize', debouncedOnResize);
document.addEventListener('scroll', windowScrollHandler, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this on Firefox? Scrolling behavior is slightly different there.

I still suspect that something is doing stopProagation on the scroll event...

Copy link

@michahell michahell Nov 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably causing the bug on firefox (at least in 1.09) where tooltips flash and immediately get removed...

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

http://www.quirksmode.org/dom/events/scroll.html

Try listening on window?

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

Hacked together a workaround:
e878047

@jelbourn
Copy link
Member

jelbourn commented Dec 8, 2015

Workaround seems like it would be good. @EladBezalel are you going to apply it and add a unit test?

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

An explanation for the workaround (which should probably be added in a comment):

It seems like appending and/or repositioning the menu was triggering the scroll events (In FF only, which seems like buggy behavior but we're stuck with it). These events happen more or less right away, so nextTick fires after them.

@EladBezalel
Copy link
Member Author

This is not working entirely, when checking the checkbox it's still disappearing.

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

Well, it's another step towards hacky, but a timeout of 100ms instead of nextTick does the trick for the checkbox. Perhaps there's a longer animation going on when it triggers from the checkbox?

@jelbourn
Copy link
Member

jelbourn commented Dec 8, 2015

What do you mean by checking the checkbox?

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

Better solution: remove transform: translate3d(0,0,0); from tooltip.scss. That seems to be the source of our FF misbehavior.

@jelbourn
Copy link
Member

jelbourn commented Dec 8, 2015

@kseamon that sounds like the best

@ThomasBurleson ThomasBurleson added this to the 1.0-rc7 milestone Dec 8, 2015
@EladBezalel
Copy link
Member Author

@jelbourn I'm on it

@kseamon
Copy link
Contributor

kseamon commented Dec 8, 2015

I have not dived in with the chrome tools to look at paint performance to see if this makes any difference, but adding "will-change: opacity, height, width;" does not trigger the misbehavior but probably replaces any performance gains that the transform was accomplishing.

@EladBezalel EladBezalel deleted the fix/tooltip-in-body branch December 9, 2015 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants