salcode / bootstrap-genesis

WordPress Genesis Child Theme setup to use Bootstrap, Sass, and Grunt
MIT License
184 stars 63 forks source link

Sidebar too narrow on sm breakpoint #108

Open salcode opened 8 years ago

salcode commented 8 years ago

@bryanwillis brought up this issue in PR #107

Specifically, he noted the problem with the content-sidebar layout.

md screenshot

salcode commented 8 years ago

@bryanwillis

I'm open to rethinking the breakpoints, e.g.

'content'          => 'col-md-8 col-lg-9',
'sidebar-primary'  => 'col-md-4 col-lg-3',

instead of col-sm-9 and col-sm-3.

I don't think I want to hide the sidebar on narrow screens by default (hidden-sm). It makes a lot of sense to hide the sidebar on some sites but I don't want to make it the default.

@dustyndoyle I know you use this theme a lot, do you have any thoughts on changing the break points like this.

dustyndoyle commented 8 years ago

@salcode I think the new breakpoints you reference col-md-8 col-lg-9 are better than keeping col-sm-9 throughout because the sidebar does get pretty small.

I would probably use col-sm-8 col-lg-9 for me that makes it not look so squished at the 991px breakpoint. Otherwise it would have the sidebar go full width on the small screen, which I am not a fan of unless absolutely necessary.

bryanwillis commented 8 years ago

Here's a sandbox/example of how I'm using it on most of my sites now:

While I had every intention of disagreeing at first about not including the hidden-sm at first, I'm coming around to the fact that it shouldn't be the "default" because then there'd most likely be the headache of constant tickets being opened with people saying things along the lines of "MYSIDEBAR IS DISAPPEARING, PLEASE HELP!"

This could be and probably should be something we put in the docs however.

If you decide not to go with hidden-sm, I'll also add it to the addon plugin (which I just fixed btw). Currently, it has the option to hide the widgets on different device widths, but I will add support for hiding sidebars as well.

screen shot 2015-10-27 at 4 09 12 pm
salcode commented 8 years ago

@bryanwillis if you want to revise PR #107 for

'content'          => 'col-md-8 col-lg-9',
'sidebar-primary'  => 'col-md-4 col-lg-3',

We can test it out. Thank you for this, I'm looking forward to testing it out.

As you suggested, I've also added a page to the Wiki on Hiding the Primary Sidebar on the XS Screen Size.

You're plugin also seems like a cool way to allow this control for hiding the sidebar on narrow screens. If you do decide to add it, we can mention that on the Wiki page.

Thank you for all your input on this.

salcode commented 8 years ago

Correction, the issue was at the sm breakpoint, not md. Issue title changed to reflect this.

salcode commented 8 years ago

I think you are both right that

'content'                   => 'col-sm-8 col-md-9',
'sidebar-primary'           => 'col-sm-4 col-md-3',

is a significant improvement.

I realized that while making this change, it makes sense to modify the other layouts as well (sidebar-content, content-sidebar-sidebar, etc.).

salcode commented 8 years ago

Branch change-cols-108 has the potential change for this default value.

For adjusting the other layouts, I've opened #119