feat(position): implement auto positioning #4899

RobJacobs · 2015-11-13T20:07:17Z

Implementing the ability to auto position elements in the position service - positionElements function. This will allow tooltips and popovers to position where they fit within the closest scrollable ancestor. As part of this PR I also documented the position service functions and refactored some of the code to make it more readable. Partially closes #4762.

icfantv · 2015-11-13T20:59:27Z

@RobJacobs, is this for the 1.2 release or were you thinking 1.0? @wesleycho?

What would you guys think about having it as a beta feature (if that's even possible).

wesleycho · 2015-11-13T21:16:27Z

This is just the $uibPosition service

wesleycho · 2015-11-14T01:36:08Z

src/position/docs/readme.md

@@ -0,0 +1,180 @@
+The `$uibPosition` service provides a set of DOM utilities used internally to absolute-position an element in relation to another element (tooltips, popovers, typeaheads etc...).
+
+#### <strong>getRawNode(element)</strong>


Not hugely opinionated, but you can get the <strong> formatting from *text*

actually **text**

wesleycho · 2015-11-14T01:45:24Z

Were you able to test this on multiple browsers?

RobJacobs · 2015-11-16T14:50:06Z

@wesleycho PR updated per comments, couple of other tweaks worth noting:

Added placementClassPrefix option to tooltip config.
Formatted readme.md to resemble dateParser doc.
Removed the 'auto' option as the default tooltip behavior so this will be an opt-in feature.

I've tested this change on the latest versions of Chrome, Firefox, Edge, IE11, IE10 & 9 using IE11 emulation mode, and on an iPad with iOS9.

icfantv · 2015-11-16T15:52:32Z

I should say, awesome work @RobJacobs.

wesleycho · 2015-11-16T16:08:27Z

src/position/position.js

+        }
+
+        while (scrollParent.parentElement && scrollParent !== documentEl) {
+          var spStyle = $window.getComputedStyle(scrollParent);


Is this the best way to do this? getComputedStyle is a pretty expensive function, and inside a while loop could be quite pricey. What happens if this never finds an absolutely positioned or position static parent element?

How many times at worst will this loop fire?

@wesleycho I was concerned about that as well, but it's the same approach the parentOffsetEl function uses in a while loop when calling the isStaticPositioned function. getComputedStyle is the only way I know of to get the styles applied by CSS and not just inline styles that element.style provides.

I could refactor and call the getStyle function instead of calling getComputedStyle directly as I only need the position style value.

I did a bit of sleuthing, and it looks like we can kill getStyle - looks like it was necessary for IE8 support (IE8 used currentStyle instead of having the built-in getComputedStyle function), but it is now unnecessary.

@wesleycho PR updated with getStyle function removed.

RobJacobs · 2015-11-16T16:09:42Z

@icfantv Thank you sir! Once/if this PR is accepted, it should be pretty straight forward to implement on the datepicker and typeahead.

wesleycho · 2015-11-16T16:29:44Z

We also want this implemented on the dropdown as well.

This is amazing work btw - I took a stab at this before, but positioning turns out to be quite formidable.

RobJacobs · 2015-11-16T16:41:53Z

Implementing this on the dropdown will present a different set of challenges as the TWBS dropdown relies more on CSS for positioning as I described in #4762, but I'm confident it is possible. Next thing I want to look at is coming up with a generic 'popup' directive that would encapsulate some of the redundant code I'm seeing:

Adding triggers
placement attribute
is-open attribute
append-to-body attribute
open/close delay

And switch datepicker, typeahead, popover, and dropdown directives to use that. Then trim down the tooltip directive to serve just that purpose - tooltips. It's getting pretty heavy trying to support all the different use cases.

wesleycho · 2015-11-16T17:06:36Z

For the dropdown, we can restrict it to being positioned up, and maybe auto with only up being applied.

wesleycho · 2015-11-18T22:35:35Z

src/position/docs/readme.md

+The `$uibPosition` service provides a set of DOM utilities used internally to absolute-position an element in relation to another element (tooltips, popovers, typeaheads etc...).
+
+### getRawNode(element)
+  Takes a jQuery/jQLite element and converts it to a raw DOM element.


I believe this should be jqLite (minor quibble)

wesleycho · 2015-11-18T22:39:53Z

This PR LGTM - just a minor doc quibble, but this is good to merge otherwise.

icfantv · 2015-11-18T23:23:24Z

Awwwww yea. @RobJacobs you should do the honors.

Foxandxss · 2015-11-18T23:26:47Z

You can merge it. I will check the docs in another moment and fix them if I feel like I have to. Sorry hadn't time to check it properly.

Foxandxss · 2015-11-18T23:29:21Z

Hold tight. If this doesn't have a demo, the documentation won't show up on the page.

RobJacobs · 2015-11-19T16:19:49Z

@Foxandxss Added demo page, @wesleycho Fixed jqLite typo. Let me know if you see any other issues.

Foxandxss · 2015-11-20T11:45:09Z

src/position/docs/readme.md

+  _(Type 'element')_ - The closest positioned ancestor.
+
+### scrollbarWidth()
+  Caclulates the browser scrollbar width and caches the result for future calls.  Concept from the TWBS measureScrollbar() function in [modal.js](https://github.com/twbs/bootstrap/blob/master/js/modal.js).


Foxandxss · 2015-11-20T11:54:58Z

About the demo, it is too technical in my opinion. I liked those demos you put to use where you can see tooltips in certain positions. That + all the technical information that you offers now is for me better.

Personally I don't like putting all that huge doc on the page, but we don't have a better mechanism for that either.

The doc itself has some nitpicks I want to change (Styling) so I would create a PR when this gets merged.

RobJacobs · 2015-11-20T13:10:47Z

@Foxandxss Fixed 'calculate' typo, updated tooltip and popover examples to showcase all available positions. Let me know if you see anything else, thanks.

Foxandxss · 2015-11-22T23:55:57Z

It is good to me. There are some fixed docs / demo related I want to do but I better do it in another PR so we don't clutter this too much.

Implementing the ability to auto position elements in the position service - positionElements function. Closes 4899

momoadeli · 2015-11-23T21:08:09Z

Apologies for the intrusion. Does this comprehensively address the popover (and tooltip) running over the edge of the page? If so, when will this be released into the wild (we are on 0.14.3 currently) and, if not, can we leverage this to address this current run-over issue once released?

icfantv · 2015-11-23T21:46:49Z

@megabyzus, while it is comprehensive, there's no way we can test it like releasing it into the wild for you guys to hammer.

It is part of master and will be in the official 1.0 release. This release is on our radar and we really want to release it by the end of the year, but as of right now we don't have a target date. If you want to start using it, feel free to pull master and run npm install followed by grunt to build your own SNAPSHOT copy. The built/generated files will be in the dist directory.

momoadeli · 2015-11-24T17:59:11Z

@icfantv Much appreciated. To address this immediately, do you have any links you can suggest to resolve this meanwhile?

icfantv · 2015-11-24T22:43:32Z

Not sure I follow, @megabyzus.

wesleycho added the component: $position label Nov 13, 2015

wesleycho added the needs: review label Nov 13, 2015

wesleycho reviewed Nov 14, 2015
View reviewed changes

RobJacobs self-assigned this Nov 16, 2015

wesleycho reviewed Nov 16, 2015
View reviewed changes

wesleycho reviewed Nov 18, 2015
View reviewed changes

Foxandxss reviewed Nov 20, 2015
View reviewed changes

wesleycho added needs: merging and removed needs: review labels Nov 23, 2015

wesleycho added this to the 1.0.0 milestone Nov 23, 2015

feat(position): implement auto positioning

d265113

Implementing the ability to auto position elements in the position service - positionElements function. Closes 4899

RobJacobs merged commit d265113 into angular-ui:master Nov 23, 2015

RobJacobs deleted the feat-auto-position branch November 23, 2015 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(position): implement auto positioning #4899

feat(position): implement auto positioning #4899

RobJacobs commented Nov 13, 2015

icfantv commented Nov 13, 2015

wesleycho commented Nov 13, 2015

wesleycho Nov 14, 2015

Foxandxss Nov 14, 2015

wesleycho commented Nov 14, 2015

RobJacobs commented Nov 16, 2015

icfantv commented Nov 16, 2015

wesleycho Nov 16, 2015

RobJacobs Nov 16, 2015

wesleycho Nov 16, 2015

RobJacobs Nov 17, 2015

RobJacobs commented Nov 16, 2015

wesleycho commented Nov 16, 2015

RobJacobs commented Nov 16, 2015

wesleycho commented Nov 16, 2015

wesleycho Nov 18, 2015

wesleycho commented Nov 18, 2015

icfantv commented Nov 18, 2015

Foxandxss commented Nov 18, 2015

Foxandxss commented Nov 18, 2015

RobJacobs commented Nov 19, 2015

Foxandxss Nov 20, 2015

Foxandxss commented Nov 20, 2015

RobJacobs commented Nov 20, 2015

Foxandxss commented Nov 22, 2015

momoadeli commented Nov 23, 2015

icfantv commented Nov 23, 2015

momoadeli commented Nov 24, 2015

icfantv commented Nov 24, 2015

		@@ -0,0 +1,180 @@
		The `$uibPosition` service provides a set of DOM utilities used internally to absolute-position an element in relation to another element (tooltips, popovers, typeaheads etc...).

		#### <strong>getRawNode(element)</strong>

feat(position): implement auto positioning #4899

feat(position): implement auto positioning #4899

Conversation

RobJacobs commented Nov 13, 2015

icfantv commented Nov 13, 2015

wesleycho commented Nov 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wesleycho commented Nov 14, 2015

RobJacobs commented Nov 16, 2015

icfantv commented Nov 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobJacobs commented Nov 16, 2015

wesleycho commented Nov 16, 2015

RobJacobs commented Nov 16, 2015

wesleycho commented Nov 16, 2015

Choose a reason for hiding this comment

wesleycho commented Nov 18, 2015

icfantv commented Nov 18, 2015

Foxandxss commented Nov 18, 2015

Foxandxss commented Nov 18, 2015

RobJacobs commented Nov 19, 2015

Choose a reason for hiding this comment

Foxandxss commented Nov 20, 2015

RobJacobs commented Nov 20, 2015

Foxandxss commented Nov 22, 2015

momoadeli commented Nov 23, 2015

icfantv commented Nov 23, 2015

momoadeli commented Nov 24, 2015

icfantv commented Nov 24, 2015