Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore collapse class when a collapse is shown, and remove dependency on panel class #11191

Closed
wants to merge 2 commits into from

Conversation

ThinTim
Copy link

@ThinTim ThinTim commented Oct 21, 2013

Fixes #10966

@ThinTim ThinTim closed this Oct 21, 2013
@ThinTim ThinTim reopened this Oct 21, 2013
@cvrebert
Copy link
Collaborator

cvrebert commented Nov 6, 2013

Could you add a unit test for the collapse class restoration?

@ThinTim
Copy link
Author

ThinTim commented Nov 6, 2013

Done. Wasn't sure whether to follow the test format of "should show a collapsed element" or "should reset style to auto after finishing opening collapse", as both seemed to work.

Let me know if I need to change it.

@FiZiX
Copy link

FiZiX commented Feb 13, 2014

Why hasn't this been committed? I'm having to patch this in every time a new version is released.

@cvrebert
Copy link
Collaborator

@FiZiX Waiting on @fat to code-review it.

@ThinTim
Copy link
Author

ThinTim commented Feb 13, 2014

@cvrebert several issues about this have been closed by @fat, including the one I submitted this request for (#10966). I assumed that meant it became a "won't fix".

@@ -541,7 +541,7 @@ if (typeof jQuery === "undefined") { throw new Error("Bootstrap requires jQuery"
this.$element.trigger(startEvent)
if (startEvent.isDefaultPrevented()) return

var actives = this.$parent && this.$parent.find('> .panel > .in')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can't be merged because this is a breaking change for some folks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we added that because the "collapsed.in" selector was to generic and people were opening a bunch of issues about nesting them etc

@fat fat closed this Mar 14, 2014
@mdo mdo removed this from the v3.2.0 milestone Mar 14, 2014
@fczuardi
Copy link

Should we update the documentation and state that for collapse to work you MUST use panels?

I think it is a shame this bug got closed as WONTFIX because there are times where a user might want to use accordions for non-panel components, like link list-group-item's…

The problem I see is that the .panel class carries a lot of visual styling with it, so if I just want to have accordion behavior on a list you will have to override the .panel stylesheet with more styling to remove stuff.

fczuardi added a commit to TelaSocial/companion that referenced this pull request Apr 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collapsible parent option is dependent on dom structure
6 participants