This repository has been archived by the owner on Sep 5, 2024. It is now read-only.
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(tooltip): corrected md-tooltip positioning when scrolled
There are several issues out there (e.g. #2406) that point to tooltip positioning when the page is scrolled and getNearestContentElement that suggests that the enclosed loop is supposed to stop at md-content but does not. Adding that condition to the loop fixed the positioning issue for me. Fixes #2406. Closes #5161.
- Loading branch information
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kasiarachwal @ThomasBurleson This broke all my tooltips and it appears to haven broken several others too. See #5161 (comment)
@kasiarachwal Do you have a demo of what situation this fixed tooltips for? In my case I have fab buttons and speed dials inside of
md-content
using tooltips. This also broke regular tooltips just inside ofmd-content
.PS this also caused another issue with tooltips seeming to have the effects of a lower
z-index
then tabs. However the z-index isn't different and I couldn't find a way using css to fix this(I think it has to deal with where the tooltip is inserted?).This also doesn't fix #2406. For me it was working with 0.11.2 and 0.11.4 at least. I'm not sure exactly when it was fixed though.
@ThomasBurleson Can you please revert this change until a better fix can be found? I'm also not sure if one is even needed as it appears to be working fine in 0.11.4. But a demo from @kasiarachwal may show that we do indeed need one.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the difference is that I have a left hand sidenav (it's already off screen here - about half a page scrolled). With the code change, when I scroll, my tooltips are correctly positioned (although I have since modified the styling).
When I put it back the way it was before the change, and I scroll, the same tooltip is not where it should be. It is roughly the sidenav's width left of where it should be and the vertical position gets worse as you scroll farther down.
I have several config screens that are arranged this way, with a left sidenav and a vertical series of collapsing/expanding card forms. This change was the only thing that allowed the tooltips to be positioned in the correct place on the screen when scrolling since 11.0.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kasiarachwal looks like the fix for #5609 will fix all of my issues.
I'm not sure if yours will work anymore though as right now
getNearestContentElement
is always selecting the root element. I'm gussing that when the sidenav is open you just need to make sure it selects the root element instead of themd-content
(or maybemd-content
s parent?).f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kasiarachwal can you provide a plunkr for your setup?
The
.toUpperCase()
fix didn't work. I found out why and I need to make a pr to fix it for both our usecases but I'd like to be able t confirm it works with your setup.As for the why it doesn't work. With/without the
toUpperCase()
to&& current.nodeName !== 'MD-CONTENT'
essentially always skipsmd-content
and uses a higher(body or html tag?) tag for the root to attatch the tooltip too. This is what should happen with your specific case(I'm gussesing whenever a sidenav is open) but when you do not have a sidenav open it should use themd-content
as the root like it did before.f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kasiarachwal I tried to replicate your setup but I don't think I have it right as they are all broken in 1.0.0-rc1+ but working in 0.11.4(your change was added just after this) just as before. I couldn't find your case in 0.11.4 where it was broken.
Demo with right sidenav locked open and tooltips on the input and the toggle right sidenav button.
http://codepen.io/anon/pen/YwKXKp?editors=101
Here is the same demo using
0.11.4
(version just before this change was added). It seems to function perfectly. Both tooltips work correctly when scrolled. Also I don't see the bug you mentioned with the tooltip being movedhttp://codepen.io/anon/pen/zrOGYO?editors=101
Here is another copy of the last demo with just a sidenav on the left(to make sure the two sidenavs weren't canceling each other out by moving the tooltip back and forth).
http://codepen.io/anon/pen/rxBVNE?editors=101
Is your setup different then this? Can you update one of them if it is?
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the last one with 1.0.0-rc5
http://codepen.io/anon/pen/QyLEJK?editors=101
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kasiarachwal any chance I can get access somehow? Still waiting to fix this.
Several more people seem to have run into broken tooltips due to this too.
@ThomasBurleson do you want to roll this back for now? It'd close quite a few issues for many people.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moot point as of 1.0.0-rc7 since the enclosing function is gone and parent is always body. Back to being broken for me.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well 1.0.0-rc7 fixed my issue.
@kasiarachwal if you can get a demo of your issue I'd be more then happy to help you find a fix that works with both use cases.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f62f693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that's good to hear it's not an angular-material issue. I ran into a similar problem with d3 breaking svg icons a while back.