-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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
Fixes #14799: Add vertical alignment of image in media component #14801
Conversation
@jenndodd Would you mind removing the dist files from the PR? They're probably the reason for the merge conflicts. |
We can do that, sure. |
Hmm, just realized @jenndodd isn't in the office today, so we'll have to hold off until we can get her to push the fix. |
Sure, no worries! |
In giving our PR one more look over, we noticed that the |
All Line 27 in 3bea2a4
It doesn't do anything else and isn't used anywhere else in the Less code. |
Signed-off-by: Jenn Dodd <jedodd@pivotal.io>
Signed-off-by: Geoff Pleiss <gpleiss@pivotal.io>
Even after reverting the dist changes we still got merge conflicts. So we modified the commits, rebased the latest changes to master, and force pushed the branch. |
Doesn't mean it can be removed though. |
All |
// Proper spacing between instances of .media | ||
.media, | ||
.media .media { | ||
.media { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the alignment for the legacy .media > .pull-left
, none of this needs to be nested under .media
. The .media
, .media-right
, .media-body
, etc can all be top level selectors here to avoid unnecessary specificity.
Our bad! We're putting |
We're getting super close to a release here, so I took care of the problems here and merged to |
Thanks! Sorry we were slow. Jenn was out of the office this week so we had to finagle some things to get new code to her fork. Nicole On Sat, Oct 25, 2014 at 9:47 PM, Mark Otto notifications@github.com
|
No worries, yo! We're really trying to get v3.3 (formerly v3.2.1) out the door. This wouldn't have happened at all without @jenndodd and you, so thanks again to y'all and sorry if I rushed anyway 😁. |
@mdo I think we missed the ship list entry for this one... |
@hnrch02 Yeah, we should edit the GitHub Release description to mention it. |
@cvrebert I'll do that. |
as was the case up to 3.2.x, prior to twbs#14801
The
.media-left
and.media-right
images can now be aligned vertically via the.media-middle
and.media-bottom
classes. Top is the default so we did not make an explicit.media-top
class.The
.pull-right/left
version still works and we left one example in the docs for testing purposes. Do you usually do that? We weren't sure how to make sure the deprecated version is still tested without keeping it in the docs.Any thoughts? We'd love feedback.
@stubbornella, @jenndodd, & @bebepeng
Fixes #14799.