Responsive containers (follow-up to #29095) #29118
Conversation
|
Thank you Mark for addressing this. Apologies if my tone was harsh. |
|
No need to apologize, I was trying to bring it over and screwed it up—and it only took me 1.5 years to agree with you. 😬 Thanks for sticking with me. |
|
@browner12 I'm pulling in something like the table you made in your original PR, but with the
@MartijnCuppens Sorry for removing your changes before, they were the right move. Let me know once you've had a chance to review this, please. |
|
yes, that looks great. makes it very clear to the user how this feature works. |
|
@browner12 Thank you! <3 |
|
Looking at the table above... both |
|
yah, below 576px it's so small that everything is just full width. |
|
With this change, |
|
Containers are either responsive or fluid. The infix determines where that switch happens. I personally think the max width feature should be handled in userland, but if you'd like to attempt a separate PR, give it a shot. |
|
Yeah, and that container |
|
With |
No problem man, nice to see you used the It's pretty weird |
|
I don't thing we could do that on v4, since that would break BC? |
|
Aha, didn't notice the |
|
Yeah @MartijnCuppens I want to make sure v4 gets this one because of how long I dragged my feet and proposed other ideas. If you could, give a once over and thumbs up if these new changes match what you and @browner12 had landed on <3. |
MartijnCuppens
changed the title
|
I was more in need of more container sizes like the initial PR, but this PR make sense too and it will probably be something more people want. For the different container sizes we can still make use of our utility API and combine I do think we should use use a So yeah, codewise this looks good to me. I do think the documentation needs more clarification though, the description here (https://deploy-preview-29118--twbs-bootstrap.netlify.com/docs/4.3/layout/overview/#responsive) is clear, but the demo is confusing. This is probably the same thing @voltaek mentioned. |
|
Will revisit docs for this tonight! |
|
We could also just drop the demo, the explanation and table are clear enough imo. The thing with the demo is that might confuse people if they start resizing their screen and notice nothing happens since we don't actually use the container classes. |
mdo
force-pushed
the
responsive-container-fixes
branch
from
3d898bb
to
b020699
Compare
Yes, I'm waiting anxiously for this. I need standard containers on a desktop where there is a ton of (too much extra) room, and fluid containers on mobile where I need every bit of real estate I can get. Thanks to @browner12 for adding it and for you guys staying on top of it. |
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.
- 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
mdo
force-pushed
the
responsive-container-fixes
branch
from
8ae2cef
to
51da4cd
Compare
* 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
* 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
* 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
* 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
This PR fixes the responsive containers that were added in #29095, originally stubbed out in #25631. Apologies to @browner12 for getting that wrong.
I still need to verify if at all we need to make changes to the navbar CSS. That looked more complicated than it needed to be, so need to verify the attribute selectors.
Fixes #25631.