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

fix border issue for nested card inside accordions #27881

Merged
merged 5 commits into from Jan 7, 2019

Conversation

ovpv
Copy link
Contributor

@ovpv ovpv commented Dec 19, 2018

After analysing the code in scss/_card.scss, in line 281, the styles added for .card class inside .accordian class is targeting all the cards inside instead of its immediate children elements with the class .card.

Code now:

.accordion {
  .card {
    ...
  }
}

Fix:

.accordion {
  > .card {
    ...
  }
}

Fixes #27510, fixes #26556.

@ovpv ovpv requested a review from a team as a code owner December 19, 2018 07:40
@MartijnCuppens
Copy link
Member

Hi @ovpv,

Can you provide a demo to make clear what this fixes?

@ovpv
Copy link
Contributor Author

ovpv commented Dec 19, 2018

The issue demo
https://codepen.io/JJCoolBean/pen/KGraWv

The fix demo
https://codepen.io/ovpv/pen/vvyRVj

@MartijnCuppens , In the above fix demo, you can see that the the border-bottom: 0 property is only applied to the accordian cards and not the child cards inside the accordian card-body.

@MartijnCuppens MartijnCuppens added this to Inbox in v4.3 via automation Dec 19, 2018
@MartijnCuppens
Copy link
Member

Hi @ovpv,

Thanks for the PR! If you open a PR which closes an issue, you can add Closes #issuenumber so that we can easily check what the PR is about. This also automatically closes the issue if the PR gets merged.

Apart from that, this looks good! Thanks for the solution.

Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@ovpv
Copy link
Contributor Author

ovpv commented Dec 19, 2018

Hi @ovpv,

Thanks for the PR! If you open a PR which closes an issue, you can add Closes #issuenumber so that we can easily check what the PR is about. This also automatically closes the issue if the PR gets merged.

Apart from that, this looks good! Thanks for the solution.

Thanks for the approval @MartijnCuppens . This is my first contribution on github! 😄

I will make sure that I add the necessary details for a PR in my future contributions 👍

@mdo mdo moved this from Inbox to Needs review/changes in v4.3 Dec 21, 2018
@XhmikosR XhmikosR changed the title fix border issue for nested card inside accordians fix border issue for nested card inside accordions Dec 22, 2018
@mdo mdo moved this from Needs review/changes to Ready to merge in v4.3 Jan 6, 2019
@mdo mdo mentioned this pull request Jan 7, 2019
@XhmikosR XhmikosR merged commit ea9129c into twbs:v4-dev Jan 7, 2019
v4.3 automation moved this from Ready to merge to Shipped Jan 7, 2019
@osnofa
Copy link

osnofa commented Sep 18, 2019

It didn't solve everything. Please try this:

<div class="container">
        <br />
        <div class="accordion">
          <div class="card">
            <div class="card-header" data-toggle="collapse" id="cardHead" href="#collapseAccordCard" aria-expanded="true">
              <a class="card-title">Accordian Title</a>
            </div>
            <div id="collapseAccordCard" class="collapse show" aria-labelledby="headingAccordCard">
              <div class="card-body">
                <div class="row">
                  <div class="col-md-4">
                    <div class="card">
                      <div class="card-body">Lorem Ipsum 1</div>
                    </div>
                  </div>
                  <div class="col-md-4">
                    <div class="card">
                  <div class="card-body">Lorem Ipsum 2</div>
                              </div>
                          </div>
                          <div class="col-md-4">
                              <div class="card">
                                  <div class="card-body">Lorem Ipsum 3</div>
                              </div>
                          </div>
                      </div>
                  </div>
              </div>
          </div> 
  </div> 
  </div> 

@XhmikosR
Copy link
Member

@osnofa you should probably make a new issue.

@osnofa
Copy link

osnofa commented Sep 20, 2019

@osnofa you should probably make a new issue.

Done. #29426

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v4.3
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

4 participants