Skip to content

Fixed Card Columns section#26587

Closed
Vimal-Raghubir wants to merge 12 commits intotwbs:v4-devfrom
Vimal-Raghubir:patch-1
Closed

Fixed Card Columns section#26587
Vimal-Raghubir wants to merge 12 commits intotwbs:v4-devfrom
Vimal-Raghubir:patch-1

Conversation

@Vimal-Raghubir
Copy link
Copy Markdown

  • Removed duplicate card section in the second column.
  • Put third card section into the details portion of the image like the first 2 columns, shifting the lorem ipsum section down into the correct spot.

#26267 defines the second point listed above. #26586 clearly defines the issues addressed in this PR.

@XhmikosR XhmikosR requested a review from mdo May 25, 2018 14:43
@Vimal-Raghubir
Copy link
Copy Markdown
Author

old look

As you can see the card column that has the black outline is a duplicate of the card column above and the red column is the one that had to be placed into the card body like the 2 sections before it.

@Vimal-Raghubir
Copy link
Copy Markdown
Author

Hi @mdo,
Any update on this PR?
Thanks.

@Vimal-Raghubir
Copy link
Copy Markdown
Author

Hi @XhmikosR , any update on this PR?

@XhmikosR
Copy link
Copy Markdown
Member

Sorry I'm very busy with work, please be patient until someone has the time to look into the changes.

Copy link
Copy Markdown
Collaborator

@andresgalante andresgalante left a comment

Choose a reason for hiding this comment

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

Hi @Vimal-Raghubir thanks for the PR, and sorry for taking us so long to get to review it.

I would like to keep more cards to show the masonry grid. Can we fix the text and keep the extra cards at the bottom of the right and middle column please?

Comment thread docs/4.1/components/card.md Outdated
</div>
<div class="card text-center">
<div class="card">
<img class="card-img" data-src="holder.js/100px260/" alt="Card image">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be card-img-top

Comment thread docs/4.1/components/card.md Outdated
<div class="card-body">
<h5 class="card-title">Card title</h5>
<p class="card-text">This card has supporting text below as a natural lead-in to additional content.</p>
<p class="card-text">This is a wider card with supporting text below as a natural lead-in to additional content. This card has even longer content than the first to show that equal height action.</p>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This card is not wider, it's taller and it's not showing equal height

@andresgalante
Copy link
Copy Markdown
Collaborator

Hi @Vimal-Raghubir Thanks for the changes, I would still like to keep more cards to show the masonry grid. Can you add them please?

thanks

@Vimal-Raghubir
Copy link
Copy Markdown
Author

Hi Andres which cards are you referring to?

mdo added a commit that referenced this pull request Jul 9, 2018
@mdo
Copy link
Copy Markdown
Member

mdo commented Jul 9, 2018

Going to tackle this in another PR with fewer changes, thanks though. See #26732.

@mdo mdo closed this Jul 9, 2018
XhmikosR pushed a commit that referenced this pull request Jul 9, 2018
XhmikosR pushed a commit that referenced this pull request Jul 12, 2018
mdo added a commit that referenced this pull request Jul 12, 2018
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