Skip to content

Conversation

@mdo
Copy link
Member

@mdo mdo commented Jul 22, 2019

Pulling this from @browner12's PR to simplify everything and document the new classes.

browner12 and others added 21 commits February 14, 2018 20:01
provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.
this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder
forgot to run my tests before the last push, i think these are better.
using the `@extend` directive I was able to clean up this code
mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)
We avoid using `@extend` whenever possible, and this is more readable
@mdo mdo merged commit 634344c into master Jul 22, 2019
@mdo mdo deleted the browner12-responsive-containers branch July 22, 2019 00:38
@mdo mdo mentioned this pull request Jul 22, 2019
@XhmikosR
Copy link
Member

@mdo this shouldn't have been merged without review...

XhmikosR pushed a commit that referenced this pull request Jul 22, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
Copy link
Contributor

@voltaek voltaek left a comment

Choose a reason for hiding this comment

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

A couple typos in navbar.scss

@include media-breakpoint-down($breakpoint) {
> .container,
> .container-fluid {
> [class^=".container"] {
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't have a period in the class name text it's looking for.

// For nesting containers, have to redeclare for alignment purposes
> .container,
> .container-fluid {
> [class^=".container"] {
Copy link
Contributor

Choose a reason for hiding this comment

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

This also shouldn't have a period in the class name text it's looking for.

@XhmikosR
Copy link
Member

Also this shouldn't have been squashed like this.

@browner12
Copy link
Contributor

I hate to say this, because I appreciate all the hard work that everyone puts into this project, but I am very disappointed with where this PR went.

I presented a PR a year and a half ago that solved an incredibly common problem, one that I've run into in almost every project I've ever done, and it's now turned into something that I, and I believe most other people, will have zero to no use for.

Containers are meant to be responsive or fluid. My PR allowed you to choose at which breakpoint this happens. I laid out an incredibly common use case. It received tremendous support.

These changes now make the containers responsive, fluid, or fixed. .container will behave differently than .container-md which will lead to confusion.

I mean no disrespect to any of you, but I believe this needs to be readdressed.

mdo added a commit that referenced this pull request Jul 23, 2019
This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.
@mdo
Copy link
Member Author

mdo commented Jul 23, 2019

@browner12 I legit screwed up bringing making changes to your PR and oversimplified the code here. I did want your original changes, and have stubbed out another PR to fix the responsive containers. See #29118.

XhmikosR pushed a commit that referenced this pull request Jul 24, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
XhmikosR pushed a commit that referenced this pull request Jul 30, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
XhmikosR pushed a commit that referenced this pull request Jul 31, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
mdo added a commit that referenced this pull request Aug 2, 2019
This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.
mdo added a commit that referenced this pull request Aug 3, 2019
This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.
XhmikosR pushed a commit that referenced this pull request Aug 4, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
mdo added a commit that referenced this pull request Aug 5, 2019
* Follow-up to #29095

This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.

* update navbar as well because we cannot reset all containers uniformly

* Update navbars example to include container-xl example to ensure containers match

* rewrite responsive containers docs, add table of max-widths

* Update container docs
- Move table up to the intro
- Remove the container example because it's actually hella confusing
- Update and link to grid example as a demo instead
XhmikosR pushed a commit that referenced this pull request Aug 6, 2019
* Follow-up to #29095

This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.

* update navbar as well because we cannot reset all containers uniformly

* Update navbars example to include container-xl example to ensure containers match

* rewrite responsive containers docs, add table of max-widths

* Update container docs
- Move table up to the intro
- Remove the container example because it's actually hella confusing
- Update and link to grid example as a demo instead
XhmikosR pushed a commit that referenced this pull request Aug 10, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
XhmikosR pushed a commit that referenced this pull request Aug 10, 2019
* Follow-up to #29095

This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.

* update navbar as well because we cannot reset all containers uniformly

* Update navbars example to include container-xl example to ensure containers match

* rewrite responsive containers docs, add table of max-widths

* Update container docs
- Move table up to the intro
- Remove the container example because it's actually hella confusing
- Update and link to grid example as a demo instead
XhmikosR pushed a commit that referenced this pull request Aug 17, 2019
* create responsive containers

provide more flexibility and allow the user to determine when containers switch from fluid to fixed width.

* fix the base container code

this commit fixes the non-media portion of the generated CSS. I learned about the `@extends` directive and was able to put it to good use.

I create a new temporary map that contains all the main `$container-max-widths` and join it to our 2 special cases of 'xs' and 'fluid'.  Then we loop through that and, with the appropriate infixes, extend our placeholder

* formatting for style

forgot to run my tests before the last push, i think these are better.

* finish incomplete comment

* fix the responsive containers

using the `@extend` directive I was able to clean up this code

* fix responsive containers in the navbar

mostly we just look through all of our breakpoints so we can include all of the responsive container classes in the tweaks we have to do for the navbar (redeclaring flex properties, don't double up on padding, etc)

* Simplify container extends

* Simplify navbar containers

* Rearrange, add comments, ensure everything is nested in $enable-grid-classes

* Reduce new CSS by using attribute selector

We avoid using `@extend` whenever possible, and this is more readable

* Update _grid.scss

* Update _navbar.scss

* Add docs for responsive containers, redesign the container layout page

* Add to the Grid example
XhmikosR pushed a commit that referenced this pull request Aug 17, 2019
* Follow-up to #29095

This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.

Fixes #25631.

* update navbar as well because we cannot reset all containers uniformly

* Update navbars example to include container-xl example to ensure containers match

* rewrite responsive containers docs, add table of max-widths

* Update container docs
- Move table up to the intro
- Remove the container example because it's actually hella confusing
- Update and link to grid example as a demo instead
lucanos pushed a commit to lucanos/bootstrap that referenced this pull request Oct 27, 2019
* Follow-up to twbs#29095

This PR fixes the responsive containers that were added in twbs#29095, originally stubbed out in twbs#25631. Apologies to @browner12 for getting that wrong.

Fixes twbs#25631.

* update navbar as well because we cannot reset all containers uniformly

* Update navbars example to include container-xl example to ensure containers match

* rewrite responsive containers docs, add table of max-widths

* Update container docs
- Move table up to the intro
- Remove the container example because it's actually hella confusing
- Update and link to grid example as a demo instead
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.

6 participants