add class scope, re-init before showing, rich triangle #6
Conversation
| @45kb: LGTM |
src/js/angular-tooltips.js Outdated
There was a problem hiding this comment.
Here i don't like, sorry, the tooltip should inherit from title attribute :) or even better attr.tooltipTitle || attr.title i think
There was a problem hiding this comment.
I change it, because title is the attribute of html. If I use title, the default title will be showing at the same time of tooltip showing. For example,
It's really annoying.
There was a problem hiding this comment.
Ok so better as i shown ;)
Just use attr.tooltipTitle || attr.title ... so that if tooltipTitle isn't available will inherit from title tag 💯
| @KynoYang thanks a lot for the PR, just see what i wrote between lines please so that we can merge. |
There was a problem hiding this comment.
There is something wrong with the caret style, i see tooltip content in lighter background color and caret is darker( caret is more black , content is more grey)
There was a problem hiding this comment.
That's my fault.
I add this because in our design, I need to set a border for the whole tooltip including the triangle. So, I use two triangle: the "before" one with larger size works as the border, the "after" one with smaller size works as the content. Just like what github does. You can check the style of github.
There was a problem hiding this comment.
Ok no problems just give to caret and content the same background ;)
There was a problem hiding this comment.
They are the same. The color of triangle is darker because of opacity.
How about set “after” no color? it makes the triangle look like what it is before.
There was a problem hiding this comment.
Yes as it was before i think it was ok.
I liked a lot the "trasparent" replacement of rgba(0,0,0,0) you did, are we sure is it supported as well and even better than rgba()?
I tried searching a lot but no results, probably trasparent is fully supported since CSS1 so better than rgba() in such cases....
| @KynoYang Ops, i didn't noticed you closed the PR, is this still valid/available? |
src/js/angular-tooltips.js Outdated
There was a problem hiding this comment.
Here, because in my project, the element which needs tooltip may change its position by mouse event. If the tooltip just init once, it will always stay in the position of first time showing.
There was a problem hiding this comment.
@45kb I also found a bug:
When I hover on the element, tooltip shows. However, if I click the element, (in my case the element will change its position) the tooltip doesn't change its position as long with the element.
I haven't solved this problem, do you have any idea?
There was a problem hiding this comment.
This is a good point and i didn't thought about it before, imho the easy way is to check somehow when the element moves on the screen and reinit tooltip, thank you
There was a problem hiding this comment.
So i would leave this part as it was before and try to check element movements somehow 👍
There was a problem hiding this comment.
But I think checking element movements costs more than initializing tooltip before showing. If you have better solution, let me know please.
There was a problem hiding this comment.
This is really heavy to catch, i agree, maybe some new attribute to specify or set when the element is going to move or change and so needs to be re-init ..... !?
Just an example:
<a tooltip tooltip-change="resize, mouseover, etc..."> I will change on resize and mouseover so re-init me when these events are fired </a>There was a problem hiding this comment.
As far as I know, resize event refers to window resize. It's not easy to know when an element resizing. 😢
Also, mouse events are not enough to detective elements movements.
There was a problem hiding this comment.
Ok not a problem at all, time for a new attribute that solves our troubles:
<a tooltip tooltip-lazy="true/false"> Lazy re-init or not, just choose </a> So that if not in lazy mode you re-init everytime ....There was a problem hiding this comment.
Better! I'll do it right now.
There was a problem hiding this comment.
Cool thank you, just leave lazy mode by default ;)
| @45kb done |
| @KynoYang great! so if you ready please commit to your repo fork so i can clone it and just test/see in browser all the changes, and we will merge it 💯 Many thanks! |
| @45kb sorry. it's too late yesterday. |
add class scope, re-init before showing, rich triangle
| @KynoYang thanks for the PR and for your time, looks everything great! i will add changes to README and create new release with your changes, thank you |
| @KynoYang here the new version https://github.com/720kb/angular-tooltips/releases/tag/0.1.5 that you can install via bower to. Thanks 👍 |
3 participants
No description provided.