Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

collapse does not toggle #971

Closed
zackarychapple opened this issue Sep 9, 2013 · 9 comments
Closed

collapse does not toggle #971

zackarychapple opened this issue Sep 9, 2013 · 9 comments

Comments

@zackarychapple
Copy link
Contributor

Bootstrap 3 has a display:none on collapse so the directive needs to be updated.

.collapse { display: none; &.in { display: block; } }
@pkozlowski-opensource
Copy link
Member

@zackarychapple BS3 support is still WIP, we are not done yet. There are 2-3 pull request for this already that needs to be reviewed. But support for collapse is probably one of the hardest things that is left.

Anyway, you can follow the progress here: https://github.com/angular-ui/bootstrap/issues?milestone=6&state=open

@caitp
Copy link
Contributor

caitp commented Sep 9, 2013

#934 and #911 are both attempts at dealing with BS3.0 collapse better (I'm working on #934, but it's still got a ways to go and I could use some suggestions on what to do in order to shrink the patch down a bit, among other things)

@zackarychapple
Copy link
Contributor Author

@caitp thanks for the Update!

@gdoteof
Copy link

gdoteof commented Oct 17, 2013

is this closed because it's working? the .6 release i have does not have collapse working

@caitp
Copy link
Contributor

caitp commented Oct 17, 2013

@gdoteof It's closed because it's a known issue. There's a big patch in #1001 made up of several different issues, combined into one, which has a working implementation. However it has not really received review and it's gotten so old that I would have to releard my way around it :(

@gdoteof
Copy link

gdoteof commented Oct 17, 2013

closing a known but not solved issue seems... like an interesting strategy.

i tried applying the 1001 patch against _bis2 but it fails the collapse tests

Expected 18 to be 0.
Error: Expected 18 to be 0.
    at null.<anonymous> (/home/g/work/vmx/bootstrap/src/collapse/test/collapse.spec.js:37:30)

@caitp
Copy link
Contributor

caitp commented Oct 17, 2013

Yeah, there seems to be a flaky test. It's been passing on Travis and every browser I've personally tested with, so it would take some investigation to see what is causing it.

If you'd like to take a look at it, by all means it could use another pair of eyes. Probably missing a $timeout.flush() somewhere, or something.

@gdoteof
Copy link

gdoteof commented Oct 17, 2013

fwiw, it Fails on Chrome 31 dev version on ubuntu. applying the patch and forcing the build is "good enough" for me at this point

thanks for the contribution

@caitp
Copy link
Contributor

caitp commented Oct 18, 2013

I've just tested on my 13.04 x64 machine with Canary, and it's all green... I think this is most likely a flakiness issue. I'll dig through in a bit and see if I'm missing a $timeout.flush(), but I thought I had caught all of them

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants