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

Describe compiled css #24271

Merged
merged 4 commits into from Oct 13, 2017
Merged

Describe compiled css #24271

merged 4 commits into from Oct 13, 2017

Conversation

ghost
Copy link

@ghost ghost commented Oct 6, 2017

Fixes #24262. Thank you @XhmikosR for the help.

Copy link
Member

@XhmikosR XhmikosR left a comment

Choose a reason for hiding this comment

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

LGTM Waiting for @mdo's word too

Copy link
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.

Thanks a lot @lucascono for the changes.

@XhmikosR I wonder if we need the table or just a list of 3 items that tells what each CSS file has. What do you think?

└── js/
├── bootstrap.js
└── bootstrap.min.js
{% endhighlight %}

This is the most basic form of Bootstrap: precompiled files for quick drop-in usage in nearly any web project. We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). CSS [source maps](https://developers.google.com/web/tools/chrome-devtools/javascript/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools.

### Comparison of CSS files

<div class="table-responsive">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Responsive tables have a different markup:
https://getbootstrap.com/docs/4.0/content/tables/#responsive-tables

### Comparison of CSS files

<div class="table-responsive">
<table class="table">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd make this table table-bordered to make it easier to read

<thead>
<tr>
<th scope="col">CSS files</th>
<th scope="col" class="w-25 text-center">Layout</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove all w-* classes

@XhmikosR
Copy link
Member

XhmikosR commented Oct 9, 2017

@andresgalante: I agree with your changes, except maybe for w-25; without it the columns don't take the whole space.

@XhmikosR
Copy link
Member

XhmikosR commented Oct 9, 2017

OK, I made the changes @andresgalante requested. @mdo: please have a look when you can.

An alternative solution would be just a list, but I feel like a table is better.

@ghost
Copy link
Author

ghost commented Oct 12, 2017

Hi, I had not responded before because of health problems. Class w-25 was added to make the width of the columns equal in a table with 100% width. Anyway it's just a question of criteria, there is no problem in changing it.

@XhmikosR XhmikosR merged commit 71e60e1 into twbs:v4-dev Oct 13, 2017
@mdo mdo mentioned this pull request Oct 13, 2017
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