Fix the max-width wrapping #128
Conversation
choldgraf
force-pushed
the
cssfix
branch
2 times, most recently
from
8377ecd to
98b2814
Compare
|
cc @hoetmaaiers |
src/scss/index.scss
Outdated
| @@ -207,7 +211,7 @@ td:first-child { | |||
|
|
|||
| .navbar-light { | |||
| background: #fff !important; | |||
| box-shadow: 0 0.125rem 0.25rem 0 rgba(0, 0, 0, 0.11); | |||
| box-shadow: 0 0.125rem 0.25rem -0.125rem rgba(0, 0, 0, 0.11); | |||
There was a problem hiding this comment.
this makes the drop-shadow on the navbar point straight down
There was a problem hiding this comment.
This makes the shadow less prominent, that is on purpose? (no strong opinion)
There was a problem hiding this comment.
It's not the intention, it was actually only necessary due to the centering (so that the top-bar didn't have shadows to the right and left, as they do now)
There was a problem hiding this comment.
How about I just strip away everything except for removing the max-width of .bd-content? I feel like that is an important bugfix that's getting bogged down by these other design questions
There was a problem hiding this comment.
How about I just strip away everything except for removing the max-width of .bd-content? I feel like that is an important bugfix that's getting bogged down by these other design questions
Yep, +1
| @@ -225,10 +229,6 @@ td:first-child { | |||
| padding: 0 15px; | |||
| } | |||
|
|
|||
| .bd-content { | |||
There was a problem hiding this comment.
I believe this is what was throwing off the width stuff
| </nav> | ||
|
|
||
| <div class="container-fluid"> | ||
| <div class="row flex-xl-nowrap"> |
There was a problem hiding this comment.
I think this was redundant, and changing it didn't seem to alter any behavior
There was a problem hiding this comment.
@choldgraf thanks a lot for looking at this!
I played a bit with the demo docs, and the issues indeed seem to be resolved with this.
I am only not fully sure about the centering included in this PR. To be clear, I think this is an improvement, but if we are going to further tweak it some more (cfr #17), I might want to get the max-width fix merged in separately (so I can use it for the pandas docs, we might do a 1.0.3 bug fix, so would be nice to also fix the docs by then).
|
@jorisvandenbossche that works for me - I'll remove the centering stuff and keep this at fixing the width issues |
|
This needs a rebase/merge now I merged the other PR |
|
ok, should be rebased...double check that it looks good! |
src/scss/index.scss
Outdated
| .container-fluid { | ||
| max-width: 1300px; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this one can still be kept? (to preserve the max-width introduced earlier)
There was a problem hiding this comment.
on wide screens, that will center the content in the middle, with the top navbar spanning the full width:
There was a problem hiding this comment.
Ah, should there be some left alignment added then (until the centering is done)?
If that gets too complicated, fine with reverting to full width for now (since the wrapping bug is annoying enough)
There was a problem hiding this comment.
I can manually remove the left margin by over-riding the bootstrap CSS, though I feel like that's a not-good practice we have learned the hard way already 😆 ...let me know what you want me to do (would prefer to figure out before I tweak things, because it is now a huge pain in the butt to edit the CSS since I have to wait for 25 seconds each time)
There was a problem hiding this comment.
@choldgraf how come you have to wait for 25 seconds each time? Are you using the webpack buid:dev flow?
There was a problem hiding this comment.
Oh yikes, I just read #127. I will look into this today!
There was a problem hiding this comment.
OK, let's then revert the max-width here. At least that fixes the main bug, and then @hoetmaaiers can work on a better solution
There was a problem hiding this comment.
Can you explain what you mean? Do you want max-width or not? Sorry, just can't tell from the language :-)
There was a problem hiding this comment.
Sorry, I meant "revert" as revert the max-width that is in master, so keeping the revert (removing the max-width) you already did here (and thus not "revert your revert" ;-))
jorisvandenbossche
changed the title
|
@jorisvandenbossche the latest commit reverts any changes other than the bugfix. I added in a comment mentioning what to do about the box shadow if we want it to point down. |
|
Thanks Chris! |
Fixes #118
I think that this should fix the weird width issues, and while I'm at it I also made the content center properly, I think.
For some reason, I think that the
.bd-content {max-width:rule was messing up our max widths and this was causing large content blocks to expand the width of the main block in the middle. This removes that, and we can control the max width of things at the.containerlevel rather than within the container (since we should be using the bootstrap CSS classes for that).It also adds a container wrapper around the navbar so that we can center it exactly as our content is centered
Check out this page in the docs, which used to break the sidebar and cause weirdly-wide content in the middle: https://186-130237506-gh.circle-artifacts.com/0/html/demo/lists_tables.html