Was pocking around in template.php and noticed there's no default case for $vars['build_mode'], so seemed like a good idea to add that in. Also, adding it as a class seems useful also.

CommentFileSizeAuthor
#2 build_mode_class-774838-2.patch1.23 KBAnonymous (not verified)
build_mode_class.patch603 byteshefox

Comments

ceardach’s picture

ceardach commented

Yeah, I actually ended up adding my own build mode variable in my custom theme, too. +1 adding this to zen.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new build_mode_class-774838-2.patch1.23 KB

This is a good idea, but doesn't fix a small bug.

The problem with the 'switch' statement is that it uses loose comparison. I'm not going to start defining arbitrary integers for my build modes, so I'm going to use strings.

When build mode is a string, $node->build_mode == NODE_BUILD_NORMAL. Constants are ideally compared with ===, but there's no way to get a switch statement to do this.

This patch lets you keep the switch statement but it double checks that $node->build_mode === NODE_BUILD_NORMAL.

Anonymous’s picture

Anonymous (not verified) commented

This patch fixes conflicts between Zen theme and potentially a handful of other modules that implement hook_content_build_modes that use strings as build mode identifiers.

See: http://drupalcontrib.org/api/search/6/_content_build_modes

Can we please please please commit this patch?

hefox’s picture

hefox commented

If NODE_BUILD_NORMAL is put right above default:, and break moved into the if statement {}, it can fallback to default cause if not NODE_BUILD_NORMAL, which I believe is desirable, right?

mlncn’s picture

mlncn commented
Status: Needs review » Reviewed & tested by the community

Tested, approved, and in use on production. Please commit!

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Component: PHP Code » PHP code
Status: Reviewed & tested by the community » Fixed

I just committed http://drupalcode.org/project/zen.git/commit/d90f68f

But, looking at this again, doesn't the current code's if ($vars['node']->build_mode === NODE_BUILD_NORMAL) mean that $vars['build_mode'] won't be set if the build mode loosely matches NODE_BUILD_NORMAL but doesn't exactly match NODE_BUILD_NORMAL?

Don't we need this?

    case NODE_BUILD_NORMAL:
      if ($vars['node']->build_mode === NODE_BUILD_NORMAL) {
        $vars['build_mode'] = $vars['teaser'] ? 'teaser' : 'full';
      }
      else {
        $vars['build_mode'] = $vars['node']->build_mode;
      }
      break;

I committed the else clause as well. http://drupalcode.org/project/zen.git/commit/6a2a1ae

Please re-open if I'm off base.

hefox’s picture

hefox commented

Yep, the else statement was needed. Thanks :)

Status: Fixed » Closed (fixed)

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

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Closed (fixed) » Fixed

We need a corresponding view mode class in D7 as well. Just added it. :-)

http://drupalcode.org/project/zen.git/commit/bfea55e

Status: Fixed » Closed (fixed)

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