Bootstrap 3.0 Collapse Update #911
Conversation
Do not apply this, as it does not work yet, but I'm working to update to use the 'collapse', 'collapsing', and 'collapse in' CSS classes. Honestly, I'm guessing as to their order of use, especially the '.collapse .in' pair. I removed the FixUpHeight function as it no longer seemed efficient to have a generic function, due to the changing CSS classes. The major issue I'm having is that the promise resolves for expand and collapse are happening at the wrong time (e.g., when it collapses the expand .then function fires; nothing happens as isCollapsed is in the wrong state). Maybe I'm doing something dumb though. :P
|
I'm still trying to wrap my mind around why Element[0].scrollHeight is being watched.. It fires when the angular bindings are resolved, and effectively does nothing that I can tell. There's just a lot here that is described as "fixing" something, but I'm not sure if that was related to the old Bootstrap, or specific browsers. |
|
collapse (at the very least, with regard to navbars) seems to be fairly broken with bootstrap 3.0 css + ui-bootstrap-tpls 0.5 (although I have not tried with 0.6 yet) (http://jsfiddle.net/fRULW/2/) -- I'd be interested in solving this in a not-hackish way, so I guess I'll do some testing with your patch and see if things can be made more sensible |
|
Okay so, TWBS3 sets the 'collapsed' class to track whether or not something is collapsed, but this isn't needed in ui-bootstrap because we can track this with js instead (but should be kept, because users might rely on it during customization) Tests fail if the initial height fix is removed, and it looks like it is doing some useful things. Maybe this could be replaced or improved upon, but I think for now it is not a massive problem (and the current fix does not get the 'expected behaviour, so that's a thing) Adding the 'in' and 'collapsed' classes seem like the real missing link between ui-bootstrap and twbs, I propose extending the 'transition' API to support some additional semantic actions, and groups of multiple triggers to make implementing these easier -- And the rest is really just ensuring that 'in' and 'collapsed' and 'collapsing' get set correctly (My patch is in progress, pretty close to the 'right behaviour' with TWBS3.0, as far as I can tell) |
Tried adding a single animationComplete function to run whenever the transition promise resolves. This *should* work, but it never fires when collapsing. :-/
|
I'm sorry, maybe it's because it's a day off, but what are you referring to when you say TWBS3.0? I'm curious to see your patch. I revised mine to be simpler, but am still having an issue with the promise failing to resolve in the collapse direction. As far as I can see, the classes change correctly on expand, but it collapses without transition. |
|
TWBS3.0 being Twitter Bootstrap v3.0.0 -- I don't think my expansion to the transition API is really needed for this stuff, but I might submit that as a separate patch anyways. Basically I'm trying to get behaviour fairly close to that in twbs v3.0.0 -- And it works pretty much perfectly, except for the initial expansion (I think this is related to the initialAnimSkip stuff) |
|
My latest code is very similar, though, to the twbs code. If I could figure out why the collapse portion of the promise isn't resolving, I believe it'd be working. |
|
Just about working perfectly here, the issue is that I get an unnecessary scrollbar on the first expand(), not too clear why yet. Short of that though, absolutely perfect behaviour as far as I can tell |
|
Awesome! Have a patch or jsfiddle I can look at? |
|
My patch is #934, still working on it. Pretty much working perfectly atm asside from the flicker for the first expand() call |
|
Hmmm, looking at your patch, this might be a better solution for some of the v3.0.0 behaviour, do you get the expected behaviour (despite the failing tests)? My patch currently involves adding some more robust mechanics for determining the dimensions of the target element, so it's pretty large and this may not be a necessarily good thing. And unfortunately, getting very broken/unusable behaviour in older versions of FF (12.8 in particular). Apart from that it seems to work like a charm, but if this is working better, then that's a big plus |
DO NOT APPLY THIS, as it does not work reliably yet, but I'm working to update to use the 'collapse', 'collapsing', and 'collapse in' CSS classes. Honestly, I'm guessing as to their order of use, especially the '.collapse .in' pair.
I removed the FixUpHeight function as it no longer seemed efficient to have a generic function, due to the changing CSS classes.
The major issue I'm having is that the promise resolves for expand and collapse are happening at the wrong time (e.g., when it collapses the expand .then function fires; nothing happens as isCollapsed is in the wrong state). Maybe I'm doing something dumb though. :P