feat(position): Access width and height for hidden fields

[caitp]

To avoid being too wordy: collapse.js needs to be able to query the height of invisible (collapsed) elements. Previous hack involved removing .collapse and re-adding it after. This is not a very good solution (although it is probably the least verbose).

jQuery provides a swapping mechanism here, which allows the height/width of hidden elements to be shown
https://github.com/jquery/jquery/blob/94ae7133449b2d0f5aab7e1a7f7190be65924145/src/css.js#L349

I propose adding height/width helpers to $position (and perhaps, just wrapping jQuery if jQuery is present), removing them from collapse.js (in #934 / #1001).

Additionally, it might be helpful to provide this behaviour for $position.offset as well, but I'm not sure.

• Pros: Lets me shrink down Fixes #911 - ui.bootstrap.collapse TWBS 3.0 compatibility (DEPENDS ON #991) #934 / feat(collapse): Bootstrap3 compatibility/bugfixes #1001 and avoid duplicating some code which is already present in $position.
• Cons: Makes library slightly bigger .___. (but very slightly!)

[caitp]

I've added some tests to this, they could be expanded a bit obviously, but whatever.

Merging this would allow me to remove a huge chunk of code from #934 / #1001 (which would make those patches much more mergeable, I think).

So, if I can't put this stuff in $position, what can I dew? Since we don't want to impose the jQuery requirement, and collapse really needs to be able to robustly determine the height/width of collapsed elements without terrible hacks, this would be a nice addition. It can easily be removed whenever angular's jqlite decides to bring in the jq dimensions routines (Hey, why not call this an 'unsupported' feature which essentially polyfills some needed functionality? then people who use it would have only themselves to blame when it's removed)

[pkozlowski-opensource]

@caitp, so, the support for collapse landed in the Bootstrap3 branch:
https://github.com/angular-ui/bootstrap/blob/bootstrap3/src/collapse/collapse.js

In this implementation the scrollHeight is used to get expanded element height. I know that you had some objections regarding this property but it seems to work "fine", BS is using it and it has advantage of being short and concise :-)

What is the exact issue with scrollHeight?

[caitp]

the gist of it is outlined in the MDN quotes posted on one of those issues.

I am not too worried about it, the main thing for me is not caring whether the ‘collapse’ class is used on the
element (really all of the DOM state stuff is kind of an issue, but I haven’t looked at your patch yet to see if
it’s been cleaned up)

Using scrollHeight, I was seeing some glitchy behaviour with FF (mistakenly believing it was not collapsed,
causing a flicker) — but I think the issue was misplaced originally (this was a while ago, so I can’t remember it
all). However, TWBS does not use scrollHeight, and MDN’s definitions of them make offsetHeight seem more
appropriate to me.

If it works, it’s not that big of a deal, though.

Caitlin Potter
[email protected]

On Nov 26, 2013, at 2:55 PM, Pawel Kozlowski [email protected] wrote:

@caitp, so, the support for collapse landed in the Bootstrap3 branch:
https://github.com/angular-ui/bootstrap/blob/bootstrap3/src/collapse/collapse.js

In this implementation the scrollHeight is used to get expanded element height. I know that you had some objections regarding this property but it seems to work "fine", BS is using it and it has advantage of being short and concise :-)

What is the exact issue with scrollHeight?

—
Reply to this email directly or view it on GitHub.