Fix regression: make media-body consume full available width #15140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
as was the case up to 3.2.x, prior to twbs#14801
|
Have any examples demonstrating the bug and fix? |
|
@mdo the original PR has links demonstrating the bug - v3.2.0 (before it broke) vs v3.3.1 (after it broke), and here's how 3.3.1 it looks with the fix: http://jsfiddle.net/jj5snj84/4/ edit: changed the examples to use |
|
Rather than go this route, I restored the styles we nuked from #14801. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The addition of #14801 made media-body not consume the full available width, due to the use of
display: table-cell. This is problematic when the media body has visible borders or elements floated to its right (see example - v3.2.0 vs v3.3.1).This PR fixes this by applying
width: 100%to media-body anddisplay: tableto the main media element, as was done in the flag object article referenced from the original feature request issue, #14799.My 2c: I don't think that applying table displays to all media elements is a good idea. Using
table/table-cellmakes elements behave quite differently and may cause things to break in odd ways.While my suggested change might fix this issue, I'm not sure that it doesn't cause others. I would much rater have vertical alignment and table display as an opt-it, either by specifying an additional class on the
mediaelement ("media-valign"?) or by making it a completely different component ("flag"?), rather than changing the behavior of an existing component and possibly breaking existing code.If you prefer, I can send a pull request that does that instead.