Skip to content

Added .display-block, .display-inline-block, .display-inline #19665

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Added .display-block, .display-inline-block, .display-inline #19665

wants to merge 4 commits into from

Conversation

teoucsb82
Copy link

Per #19443
Cleans up lingering doc/scss references to .center-block (post #19102)

Per #19343
Added new .display-<display-type> classes

Request was to combine them in one PR, but they're in two commits if either feature is wanted without the other.

cvrebert
cvrebert reviewed Apr 2, 2016
View reviewed changes
scss/utilities/_display.scss
//

.display-block {
display: block;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Utilities should use !important

@cvrebert
Copy link
Collaborator

cvrebert commented Apr 2, 2016

Given the naming of some of the other utilities, perhaps these should be .d-inline-block or even .d-ib.
@mdo Your thoughts?

cvrebert
cvrebert reviewed Apr 2, 2016
View reviewed changes
docs/content/images.md
@@ -60,11 +60,11 @@ Align images with the [helper float classes]({{ site.baseurl }}/components/utili
{% endhighlight %}

<div class="bd-example bd-example-images">
<img data-src="holder.js/200x200" class="img-rounded center-block" alt="A generic square placeholder image with rounded corners">
Copy link
Collaborator

Choose a reason for hiding this comment

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

The uses of .center-block need to be replaced with .m-x-auto (and adding .display-block might be necessary too).

@teoucsb82
Copy link
Author

Thanks for the feedback, made adjustments. HoundCI is dinging me for the !importants on a utility class, please let me know if there's anything I can do to get that passing? Also renamed the display classes to .d-block, .d-inline-block, etc based on your comment, seemed the most readable but can adjust as needed.

cvrebert
cvrebert reviewed Apr 3, 2016
View reviewed changes
docs/examples/carousel/index.html
@@ -129,7 +129,7 @@ <h2 class="featurette-heading">First featurette heading. <span class="text-muted
<p class="lead">Donec ullamcorper nulla non metus auctor fringilla. Vestibulum id ligula porta felis euismod semper. Praesent commodo cursus magna, vel scelerisque nisl consectetur. Fusce dapibus, tellus ac cursus commodo.</p>
</div>
<div class="col-md-5">
<img class="featurette-image img-fluid center-block" data-src="holder.js/500x500/auto" alt="Generic placeholder image">
<img class="featurette-image img-fluid m-x-auto" data-src="holder.js/500x500/auto" alt="Generic placeholder image">
Copy link
Collaborator

Choose a reason for hiding this comment

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

So these examples render okay without .d-block ?

Copy link
Author

Choose a reason for hiding this comment

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

Correct, these rendered as expected w/o need for additional .d-block (same for line 90 in docs/index.html). Only the docs/images.md needed it to match previous styling.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great!

cvrebert pushed a commit that referenced this pull request Apr 6, 2016
@cvrebert
Add display utility classes (.d-{inline,block,inline-block}); fixes #…
1b35105 
…19343

Refs #19665
[skip sauce]
@cvrebert
Copy link
Collaborator

cvrebert commented Apr 6, 2016

Merged the display utilities addition as 1b35105.

cvrebert pushed a commit that referenced this pull request Apr 6, 2016
@cvrebert
Finish killing .center-block; fixes #19443
a0a157d 
Replace the leftover instances with .m-x-auto (and .d-block when necessary)
Closes #19665
Refs #19102
[skip sauce]
@cvrebert
Copy link
Collaborator

cvrebert commented Apr 6, 2016

Merged the .center-block removal as a0a157d.

Thanks!

@cvrebert cvrebert closed this Apr 6, 2016
@cvrebert cvrebert added this to the v4.0.0-alpha.3 milestone Apr 6, 2016
cvrebert added a commit that referenced this pull request Apr 7, 2016
@cvrebert
Update lingering implicit prose reference to .center-block
835676b 
Closes #19339
Refs #19102, #19443, #19665
[skip sauce]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
css docs feature v4
Projects
None yet
Milestone
v4.0.0-alpha.3
Development

Successfully merging this pull request may close these issues.
2 participants
@teoucsb82 @cvrebert