Quoting joachim on the main thread:

Only one sidebar now? How easy is it to modify the theme to put it on the left instead of the right? If we're aiming to be easily modifiable, changing sidebars from side to side is something people will want to do.

http://drupal.org/node/683026#comment-2542558

I'm calling this a bug because I think otherwise Bartik's too prescriptive to be the default theme.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

I've committed a fix to this on github.

willmoy’s picture

willmoy’s picture

Project: Drupal core » Bartik
Version: 7.x-dev »
Component: other » Code
Priority: Normal » Critical
Issue tags: -Bartik

Now we have a bartik project, moving there. Marking as critical because I assume it blocks bartik in core.

willmoy’s picture

Status: Active » Needs review

Marking as needs review because joachim has code for this.

yoroy’s picture

Version: » 7.x-1.x-dev
FileSize
1005 bytes

I tried to recreate this as a patch. Can't read patches well so got confused with the part in layout.css, is there a link on that github page that exports the fix as a patch?

Work in progress patch, needs review for bot, then needs work to add the changes in layout.css

joachim’s picture

Title: Add either a second sidebar or the ability to choose which side the sidebar appears » Add a second sidebar
FileSize
2.93 KB

Here's a proper patch of the whole change.

Changing issue name too.

As I remarked in the main Bartik issue, there remains a problem with the full widths across the page not adding up which my patch does not address as I'm not sure what jensimmons' intentions are here.

emmajane’s picture

Status: Needs review » Needs work
FileSize
117.52 KB
131.83 KB
114.87 KB
76.99 KB
76.96 KB

The patch is fantastic. Screenshots attached #1,2,3.

HOWEVER, when the "display content in hidden regions" is turned on, the layout fails miserably. See attached screenshots #4,5.

If this doesn't matter because people should just turn that bit off, please mark the patch "reviewed and tested by the community" (which means Jen should roll it into the main theme).

If this DOES matter (which it probably does?) then the patch will need to be updated to fix the Lorem ipsum block.

jensimmons’s picture

We will be removing the dummy content as per webchick's request — so we don't need to fix the layout with that content.

mikey_p’s picture

This should fix the git prefix issue

jensimmons’s picture

I'm still getting errors for this patch.

patching file bartik.info
Hunk #1 succeeded at 18 (offset -1 lines).
patching file css/layout.css
Hunk #2 FAILED at 113.
1 out of 2 hunks FAILED -- saving rejects to file css/layout.css.rej
patching file page.tpl.php

I'm going to try and roll it from scratch.

chx’s picture

FileSize
0 bytes

Rerolled against HEAD

chx’s picture

FileSize
3.32 KB
jensimmons’s picture

Status: Needs work » Reviewed & tested by the community

That chx reroll works great. Thanks everyone.

As soon as I figure out how to properly commit a patch, I will.

jensimmons’s picture

Status: Reviewed & tested by the community » Fixed

Committed!
(my first cvs commit)(ever)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.