As far as I can tell, the method for changing class_content, class_sidebar_first, and class_sidebar_second recommended in example.template.php.txt is always ignored. Since BASETHEME_preprocess_page is fired after SUBTHEME_preprocess_page, squaregrid_preprocess_page should check to see if overrides have been set before setting widths.

CommentFileSizeAuthor
#1 squaregrid-fix_SUBTHEME_preprocess_page_overrides-1850250-1.patch3.29 KBianwremmel

Comments

ianwremmel’s picture

ianwremmel commented
StatusFileSize
new squaregrid-fix_SUBTHEME_preprocess_page_overrides-1850250-1.patch3.29 KB

This patch should fix the problem, but should perhaps be expanded as, once in place, the UI can no longer be used to change push/width settings in subthemes that use template.php for such overrides.

That said, does it make sense to discourage subthemers from using template.php for this purpose and instead encourage them to use SUBTHEME.info?

laura s’s picture

laura s commented
Assigned: Unassigned » laura s

Thanks for the report. I'll look this over and ponder options. We definitely are in agreement that themers should be able to override the UI-configurable settings.

laura s’s picture

laura s commented
Component: Code » Miscellaneous
Category: bug » support

I was unable to replicate the issue you describe. Here's what I did:

  1. Copied the example code into my template.php file.
  2. If I did not already have a preprocess_page function in the site where I'm trying to replicate the issue you describe, I would have renamed the "YOURTHEME" part of the function name to my child theme name. However, as I already have a preprocess_page function in my site, I omitted the entire line with the function name and added the lines to my function.
  3. Refreshed caches ("drush cc all" or "Flush all caches" using devel module).

My overrides immediate took.

Could you describe what you're doing? Did you set your grid width settings for your child theme?

laura s’s picture

laura s commented
Status: Active » Postponed (maintainer needs more info)
ianwremmel’s picture

ianwremmel commented
Status: Postponed (maintainer needs more info) » Active

Are you asking if I set the grid widths in the child theme info file or the child theme settings via the UI?

I probably won't be able to verify anything until at least Saturday, but in either case, I believe setting a grid width will take precedence over the values I assign via template.php.

In my child theme's template.php, I placed the following in spelc_preprocess_page:

  if ($page['sidebar_first'] && $page['sidebar_second']) {
    $variables['class_content'] = t('sg-17');
    $variables['class_sidebar_first'] = t('sg-10 push-18');
    $variables['class_sidebar_second'] = t('sg-6 push-29');
  }

  // if only first sidebar is present
  if ($page['sidebar_first'] && !$page['sidebar_second']) {
    $variables['class_content'] = t('sg-24');
    $variables['class_sidebar_first'] = t('sg-10 push-25');
  }

  // if only second sidebar is present
  if ($page['sidebar_second'] && !$page['sidebar_first']) {
    $variables['class_content'] = t('sg-24');
    $variables['class_sidebar_second'] = t('sg-10 push-25');
  }
 
  // if no sidebar is present
  if (!$page['sidebar_first'] && !$page['sidebar_second']) {
    $variables['class_content'] = t('sg-35');
  }

Those values get overwritten by squaregrid_preprocess_page because, as far as I can tell, spelc_preprocess_page is getting called before squaregrid_preprocess_page.

As I said, I'll try to confirm this is the behavior I'm seeing on Saturday, but hopefully this info helps.

laura s’s picture

laura s commented

With the 3.x branch, you actually don't need to set grid widths in code. I suggest trying removing that code from your child theme's template.php, and setting the width and push values in the child theme's settings page.

My testing shows that preprocess in the child theme's template.php overrides preprocess of the same thing in the base theme's template.php, so I am unable to replicate what you're reporting. It sounds like your code isn't being read at all.

ianwremmel’s picture

ianwremmel commented

Hi Laura, sorry for taking so long to reply. I saw the changes that you made to the documentation indicating that widths should be set from the theme's settings page. While it's certainly convenient in some cases to set widths from there, I'd like to be able to set the widths from my code so I don't have to set them manually or use Strongarm and Features to make my settings deployable (I tried overriding the defaults in my THEME.info file, but that didn't seem to have any effect).

To convince myself I'm not crazy and figure out which hook fires first, I put the following code at the top of squaregrid_preprocess_page and THEME_preprocess_page:

  dpm(date(DateTime::W3C), __FUNCTION__);
  sleep(3);

The result was:
spelc_preprocess_page => 2012-12-08T10:03:07-05:00
squaregrid_preprocess_page => 2012-12-08T10:03:10-05:00

As you can see from the timestamps, THEME_preprocess_page gets fired first, so anything set there will be overwritten by the base theme.

I'm not saying my solution above was ideal (it ignored many of the variables set in the info file and on the settings page), but I think increased flexibility in being able to set theme settings from code would make subtheming (and subsequent deployments) easier. If you're interested, I'd be happy to work on a cleaner method than my original patch to support both methods.