It would be fantastic if we could change our sidebar widths with through a sub-theme's admin interface.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

zilla’s picture

that would be very interesting - like a view of possible layouts (one sidebar, no sidebars, two sidebars) and then a simple text entry for something at that layout level like span4 for left column, span8 for main content (etc), or maybe additional for maxwidth and so on

mattsmith3’s picture

To start, it could be as easy as 3 dropdowns with 1-12 as column values, for the left, content, and right regions. A quick check to make sure they add up to 12, and we're on the way.

netikseo’s picture

I also think it would be very useful...

joep.hendrix’s picture

+1
Currently we have set in code the nr of spans per sidebar en the preprocess_page will generate the classes accordingly.

tkruger’s picture

I would like to see this feature as well. I need first_sidebar to be 4 column and if the sidebar is present have the middle content be corrected.

markhalliwell’s picture

Title: Change Sidebar Widths in Settings » Provide setting to change sidebar/content widths

  • Commit 7b4220c on 7.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Assigned: Unassigned » ryan.armstrong
Status: Active » Needs review

This feature is quite extensive and adds the necessary settings to create "dynamic regions". Please test that this indeed functions correctly (desired).

NOTE: While coding this feature, I realized that there has been extraneous markup surrounding regions. Several new region--*.tpl.php files have been added (navigation, header, content, sidebars, footer) and their subsequent page.tpl.php code moved into these. You will likely need to convert existing sub-themes to this new architecture.

Christopher Riley’s picture

I am curious as to why you decided that we need to have the menus rolled into $variables['page']['navigation'] as I am afraid that it is going to cause some people grief on customization.

markhalliwell’s picture

Version: 8.x-3.x-dev » 7.x-3.x-dev
Assigned: ryan.armstrong » Unassigned
Status: Needs review » Needs work
Related issues: +#1893532: [bootstrap][policy][7.x-3.x] Navigation/Menus
  1. To get the region columns/dynamic regions working properly, everything needed to be moved into regions. This helps keep the format consistent and eliminates things like $navbar_classes.
  2. For regions to be rendered, they must have content. So the primary and secondary navs were moved into a region (so the region container, with a potential column class, could wrap it).

TBH, I'm not entirely "thrilled" with this approach and agree that throwing this in there is probably not the best idea. I was just trying to commit something while not actually breaking it either. The "primary and secondary navs" are actually legacy code. Do we want to spend the time and refactoring it? Should we try and tackle #1893532: [bootstrap][policy][7.x-3.x] Navigation/Menus before hand?

This is really just the first pass at making this feature a reality (and what makes the most sense ATM). I certainly welcome ideas, thoughts and suggestions, which I should have stated before when asking to test.

bkno’s picture

Updated my subtheme to use the new approach, works great - thanks!

  • Commit d516e9e on 7.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Assigned: Unassigned » ryan.armstrong
Status: Needs work » Needs review

Ok, I moved it back into the page variables (like it was before). I just had to extend the if statement to include other things than just "$content" so it'd print the region.... missed that last go around.

  • Commit 03abb1a on 7.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Regions suggestions now include "front" and entity specificity (if the menu object is an entity):

region--sidebar--front.tpl.php
region--sidebar--node--article.tpl.php
region--sidebar--node--23.tpl.php
region--sidebar--user.tpl.php

  • Commit 03abb1a on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • Commit 7b4220c on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • Commit d516e9e on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
TheJoker’s picture

This is how it could be due to the fact that my own regions in the subtopic not disappear at variable: if (!empty($page['line']) || !empty($page['content_last'])):

???

They are only on the front page, but regions are displayed even if there are no other pages, respectively, they display a blank code without content. I'm confused.

  • Mark Carver committed 03abb1a on 8.x-3.x.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • Mark Carver committed 7b4220c on 8.x-3.x.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • Mark Carver committed d516e9e on 8.x-3.x.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Version: 8.x-3.x-dev » 7.x-3.x-dev
Assigned: ryan.armstrong » Unassigned
Status: Needs review » Needs work
Related issues: +#2311991: [bootstrap][policy] Figure out versioning

It has become increasingly obvious that this needs to either be rolled back (and wait for BS4, aka: major version bump: 7.x-4.x) or figure out a better solution that doesn't break backwards compatibility. Any suggestions?

  • Mark Carver committed cb34c92 on 7.x-3.x
    Issue #2128129 by mattsmith3: [revert] Provide setting to change sidebar...
markhalliwell’s picture

Status: Needs work » Needs review
FileSize
31.25 KB

Here is the reverted changes as a patch. We cannot, for obvious backward compatibility/update reasons, add this into the 7.x-3.1 release as it stands now. If someone wants to try and take a stab at this so it doesn't drastically alter the page and region templates, please go for it. That being said, I do think this is the right approach, we may just have to wait for BS4 (aka 7.x-4.x).

markhalliwell’s picture

I have unpublished the change record (to be a draft). I also placed a note at the top explaining that this is currently experimental code and that users should see this issue for further details.

markhalliwell’s picture

More back history for why this has been reverted.

Robytfc’s picture

Could this be something added in 8.x? if so, would there be interest in a patch?

  • markcarver committed 03abb1a on 7.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • markcarver committed 7b4220c on 7.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • markcarver committed cb34c92 on 7.x-4.x
    Issue #2128129 by mattsmith3: [revert] Provide setting to change sidebar...
  • markcarver committed d516e9e on 7.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Version: 7.x-3.x-dev » 7.x-4.x-dev
Status: Needs review » Postponed

@neardark and I discussed this at length in #drupal-bootstrap.

Because of the nature of this issue and the amount of code it changes, it is impossible to successfully commit this patch for BS3 (7/8.x-3.x) due to backwards compatibility issues mentioned above and how it may impact over nearly 50k installs.

This feature is definitely on the books to make it into BS4 (7/8.x-4.x). If one needs this support for the 3.x branches, the above patch does work (I use it myself on a couple projects), but it is too big of a paradigm shift in how pages and regions are constructed to introduce into a stable 7/8.x-3.x release.

As far as a release date for 7/8.x-4.x, that remains to be determined. No version of BS4 (alpha or beta) has yet been released. Once one does, I imagine this will become more of our focus and we can pick this issue back up.

markhalliwell’s picture

@Robytfc, I didn't answer your question entirely though. Yes, a patch for 8.x would certainly be welcomed. It won't be committed until BS4 is a reality though. So if one for 8.x-3.x is needed, it will only ever live here.

  • markcarver committed 03abb1a on 8.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • markcarver committed 7b4220c on 8.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
  • markcarver committed cb34c92 on 8.x-4.x
    Issue #2128129 by mattsmith3: [revert] Provide setting to change sidebar...
  • markcarver committed d516e9e on 8.x-4.x
    Issue #2128129 by Mark Carver | mattsmith3: Provide setting to change...
markhalliwell’s picture

Version: 7.x-4.x-dev » 8.x-4.x-dev

Changing to proper branch version for #2554199: Bootstrap 4.

markhalliwell’s picture

FileSize
32.21 KB

Here's a rerolled patch against the latest 7.x-3.x branch (for 7.x-3.0-beta1 installs, so they can "upgrade").

nithinkolekar’s picture

review

1- small typo in templates/system/region--header.tpl.php
<?php if ($content): ?>\

2- Notice: Undefined variable: regions in line 134 of theme-settings.php which makes Region column size and Dynamic regions form not rendering in theme settings page.

3- enabling Fluid container doesn't work anymore for navbar i.e menu is rendering in 12 column fixed size.

nithinkolekar’s picture

after correcting (2) theme settings are rendering properly, but I do not get how this "Region column size" works.

Suppose if we set 3 column setting to 2 column will that change "Side first" or any other region having 3 columns to 2 columns?

navbar screens

working before
fluid-container-working

not working with patch applied
fluid-container-not-working

nithinkolekar’s picture

@markcarver
any reason id="navbar-collapse" is removed from bootstrap/templates/system/region--navigation.tpl.php?

Nav menu is not expanding in mobile device without that id.(got hint from #2277725-3: Dropdown menu is not expanding on mobiles with bootstrap 3 theme)

shelane’s picture

Status: Postponed » Closed (won't fix)

This theme will not be supported for Bootstrap 4. See alternative themes for this support.