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

Prevent collapsed style overflow: hidden from obscuring the typeahead co... #994

Closed
wants to merge 2 commits into from

Conversation

johnklehm
Copy link

...ntrol's suggestion popup. I ran into the issue after having a typeahead control as a child of a div that could be collapsed.

@caitp
Copy link
Contributor

caitp commented Sep 12, 2013

I would suggest not passing the 'isCollapsed' state, but instead checking element.hasClass('collapse') -- because it is possible for isCollapsed and the classList to get out of sync and confused, unfortunately.

That said, this is sort of a part of what I'm doing in #934 and I'd appreciate any comments you might have there (currently it depends on #991 to have not-broken behaviour in FF4-16 and possibly other browsers ._., and still needs the test suite to be updated, and perhaps could be made slightly more concise). I'm all ears if you have any ideas.

@johnklehm
Copy link
Author

@caitp Didn't know about the possible loss of sync between the states but seems like that might be a separate bug from this issue? Using hasClass everywhere pushes our source of truth into the DOM though which feels wrong to me. I like to keep the source of truth in the javascript model. Is using the DOM as the source the preferred way in angular-ui? If so I'll gladly send an updated patch.

@caitp
Copy link
Contributor

caitp commented Sep 13, 2013

Because the code currently supports cancellation of transitions (this is a bug, as far as TWBS compat is concerned), you can have situations where this will result in incorrect behaviour. Removing cancellation would fix this, sort of, but all of these things are sort of tied into #1001 and solving the collapse transition stuff.

@mvhecke
Copy link
Contributor

mvhecke commented Oct 14, 2013

What is the status on this pull request?

@caitp
Copy link
Contributor

caitp commented Oct 14, 2013

#1001 has a working collapse with strong cross-browser support/testing, which is awaiting review (but has been used in a few production apps, at least).

It's been a while since I've looked at it, so I could probably get a fresh perspective on it if I took a look at it now. But self-review doesn't mean a whole lot, so that's a thing. Looks like this one hasn't received a proper look either. Anyways, this is for the master branch and not bootstrap_bis2, so I guess if this makes BS2.3 happier, that's awesome.

@pkozlowski-opensource
Copy link
Member

@johnklehm hmm, just looked into it and Bootstrap, in 2.3, is not removing the collapsed style but rather is adding the in class when the element is expanded. Well, in reality there is a bit more magic going on here and I'm not sure I understand all the steps taken by the original Bootstrap code....

@caitp it looks like the logic in 2.3 and 3.0 is different, 3.0 is using additional collapsing class. I'm really bad at those CSS tricks so I could really use some help here... Please ping me if you would be willing to give me a hand here.

@pkozlowski-opensource
Copy link
Member

Alternative solution: #828

@pkozlowski-opensource
Copy link
Member

It was fixed in master via 9eca35a. The idea is to add the in class so you can style expanded panels to your liking.

@johnklehm
Copy link
Author

Thanks!

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

Successfully merging this pull request may close these issues.

4 participants