720kb / angular-tooltips

Angularjs tooltips module, add tooltips to your elements - https://720kb.github.io/angular-tooltips
351 stars 157 forks source link

Add support for corner positions. #206

Closed leonard-thieu closed 7 years ago

leonard-thieu commented 7 years ago

Fixes #205

This PR adds support for corner positions (top left, top right, bottom left, bottom right). This also modifies the smart option to include the corner position as candidates. However, it appears the smart algorithm doesn't work correctly even in the most recent commit (44da315a24451b04b2d59d38569c1f9f20e4b436). I believe this also fixes a bug with onTooltipSideChange where it may have failed to remove the old attribute.

Also, I didn't know what to do with the tip arrows. They ended up looking pretty ugly.

45kb commented 7 years ago

@leonard-thieu tried it and looks working, but atm is not valid to be merged unfortunately.

Can you make these changes?

thanks a lot for your time!

leonard-thieu commented 7 years ago

:not() looks not widely supported http://caniuse.com/#search=%3Anot

That's for the selector list argument. I'm only using single simple selectors.

The tip arrow is not showing up

I'm not sure why the tip arrow is not showing up for you (or why the tooltip isn't positioned correctly). I can't reproduce it. This is how it displays for me:

corner-positions-browsers

leonard-thieu commented 7 years ago

Also, I'm thinking I should change the order of the directions that the smart algorithm iterates through. Originally, smart selected counter-clockwise from the start direction (except for bottom for which it went clockwise). I tried to keep some backwards-compatibility by having it try cardinal directions first but I didn't implement it correctly (and it wouldn't work for bottom anyway).

I think it might make more sense to have it cycle through all directions in order (i.e. top -> top left -> left -> bottom left). This should result in it selecting a position closer to the start position and take less iterations to find a position.

45kb commented 7 years ago

@leonard-thieu

That's for the selector list argument. I'm only using single simple selectors.

You're right sorry i found the wrong :not()

I'm not sure why the tip arrow is not showing up for you (or why the tooltip isn't positioned correctly). I can't reproduce it.

Now it works also for me, i cloned your fork https://github.com/leonard-thieu/angular-tooltips and checked out the corner-positioins branch, everythings ok now :)

So let me know how you want to proceed for the directions order, if to open a new PR and merge this or if to wait a little, no problems ... :)

leonard-thieu commented 7 years ago

I'll just make the change now since it hasn't been merged yet.

leonard-thieu commented 7 years ago

This should be good to go.

45kb commented 7 years ago

schermata 2017-05-21 alle 08 52 35 @leonard-thieu tried it locally https://github.com/leonard-thieu/angular-tooltips/tree/corner-positions

but it doesn't seem to work, am i missing anything?

thank you.

leonard-thieu commented 7 years ago

angularjs_tooltips_-_google_chrome_2017-05-21_08-26-23 I can't reproduce and I'm not sure why it's positioning top instead of top left for you.

45kb commented 7 years ago

@leonard-thieu sorry my bad this was the problem https://github.com/720kb/angular-tooltips/blob/master/index.html#L359 :) must be src/ not dist/ thanks for this great addition, i am going to release it

45kb commented 7 years ago

shipped 📦 https://github.com/720kb/angular-tooltips/releases/tag/1.2.0