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

Convert Collapse accordion from Panels to Cards #18400

Merged
merged 1 commit into from
Oct 11, 2016
Merged

Convert Collapse accordion from Panels to Cards #18400

merged 1 commit into from
Oct 11, 2016

Conversation

Johann-S
Copy link
Member

@Johann-S Johann-S commented Dec 1, 2015

Hi,

Fix for #18375

But with this PR Collapse plugin is now dependent of cards, So if you're ok with that I'll work on how to remove this dependency.

A preview from the result :

collapse

@cvrebert
Copy link
Collaborator

cvrebert commented Dec 1, 2015

Could you update the Accordion in js/tests/visual/modal.html too please?

@cvrebert
Copy link
Collaborator

cvrebert commented Dec 1, 2015

But with this PR Collapse plugin is now dependent of cards, So if you're ok with that

Yeah, that's fine. We can leave removing the dependency to a future pull request.

CC: @mdo for review

@cvrebert cvrebert changed the title [V4] Collapse using card component Convert Collapse accordion from Panels to Cards Dec 1, 2015
@Johann-S
Copy link
Member Author

Johann-S commented Dec 2, 2015

Done for the modal.html collapse.

Thank you for your feedbacks

@Johann-S Johann-S mentioned this pull request Dec 16, 2015
8 tasks
@cvrebert
Copy link
Collaborator

Overlaps with #17159.

@@ -19,6 +19,10 @@
margin-bottom: $card-spacer-y;
}

.card-title > a {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, in general, we're trying to avoid tag selectors and child selectors.

@Johann-S
Copy link
Member Author

I did changes 👍

+ '</div>'
var showFired = false
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

@Johann-S
Copy link
Member Author

Johann-S commented Sep 8, 2016

Since the merge of #17159 by @mdo I rebased my PR.

@Johann-S
Copy link
Member Author

Johann-S commented Sep 9, 2016

This is why I created a class .card-collapse .

With .card-collapse :
collapse_card-collapse

With .card-block :
collapse_card-block gif

/CC @cvrebert @mdo

@mdo mdo added this to the v4.0.0-alpha.5 milestone Oct 10, 2016
@mdo
Copy link
Member

mdo commented Oct 10, 2016

Hey @Johann-S, I missed this completely. Can you update this PR to resolve the conflicts? Would love to get this merged :). I'd also ask that you use the changes I made in #20870 so we can avoid the addition of .card-collapse entirely.

@Johann-S
Copy link
Member Author

Sure I'll work on it !

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '<div class="panel"/>'
+ '<div class="panel"/>'
+ '<div class="panel"/>'
var accordionHTML = '<div id="accordion">'

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '<div class="panel"/>'
+ '<div class="panel"/>'
+ '<div class="panel"/>'
var accordionHTML = '<div id="accordion">'

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

@Johann-S
Copy link
Member Author

Thank you @mdo for what you've done on #20870.
And it's done I've made the changes 👍

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '<div class="panel"/>'
+ '<div class="panel"/>'
+ '<div class="panel"/>'
var accordionHTML = '<div id="accordion">'

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

+ '</div>'
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.panel')
var $groups = $(accordionHTML).appendTo('#qunit-fixture').find('.card')

Choose a reason for hiding this comment

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

Unexpected var, use let or const instead no-var

@mdo
Copy link
Member

mdo commented Oct 11, 2016

Do we need to silence these Hound errors or is there something else we need to change?

@mdo
Copy link
Member

mdo commented Oct 11, 2016

Oh wait, sorry missed the file that's in—we're working on the test files elsewhere I think. Cool!

@mdo mdo merged commit 97b9d12 into twbs:v4-dev Oct 11, 2016
@Johann-S Johann-S deleted the collapseCard branch October 11, 2016 20:25
@Johann-S
Copy link
Member Author

I think @mdo you forget this PR in the v4 Alpha 5 Ship list (#20630) or I'm wrong ?

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

Successfully merging this pull request may close these issues.

4 participants