follow up #2009580: Replace theme() with drupal_render() in image module

Need convert theme('image_resize_summary', $variables) to drupal_render() function.

to test this code

  1. Go to the image styles admin area (admin/config/media/image_styles)
  2. Edit the thumbnail image style
  3. make sure it has a crop action (you may need to add one instead of scale)
  4. check that the preview of the image style (on the right) is rendering correctly
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helga.cheberakha’s picture

Assigned: Unassigned » helga.cheberakha
Issue tags: +CodeSprintUA

WDG (Ukraine,Kharkov) want to implement this on CodeSprintUA.

helga.cheberakha’s picture

Samvel’s picture

Status: Needs review » Needs work
 $image_crop = array('#theme' => 'image_resize_summary',);

Please remove "," from the end

helga.cheberakha’s picture

Assigned: helga.cheberakha » Unassigned
Status: Needs work » Needs review
FileSize
585 bytes
podarok’s picture

Status: Needs review » Reviewed & tested by the community

#4 looks good
if bot happy - i`m happy

Status: Reviewed & tested by the community » Needs work
Issue tags: -CodeSprintUA
JeroenT’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +CodeSprintUA
JeroenT’s picture

Status: Needs work » Needs review
FileSize
647 bytes

replace theme() with drupal_render in theme_image_crop_summary().

JeroenT’s picture

Patch #9 is wrong. This is the right patch.

thedavidmeister’s picture

Status: Needs review » Needs work

Patch in #10 has trailing whitespace.

JeroenT’s picture

Status: Needs work » Needs review
FileSize
663 bytes

Fixed whitespace.

eromero1’s picture

Status: Needs review » Reviewed & tested by the community

Everything ran smoothly with the patch. The crop effect worked properly, and there were no observable errors

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/image/image.admin.incundefined
@@ -748,7 +748,14 @@ function theme_image_scale_summary($variables) {
-  return theme('image_resize_summary', $variables);
+  $image_resize_summary = array(
+    '#theme' => 'image_resize_summary',
+    '#data' => array(
+      'width' => $variables['data']['width'],
+      'height' => $variables['data']['height'],
+    ),
+  );
+  return drupal_render($image_resize_summary);

I don't get why this is not...

  $image_resize_summary = array(
    '#theme' => 'image_resize_summary',
    '#variables' => $variables
  );
  return drupal_render($image_resize_summary);

The other changes here look out-of-scope to me.

heddn’s picture

Status: Needs work » Needs review
FileSize
1016 bytes

Let's try this.

star-szr’s picture

Status: Needs review » Needs work

#variables won't work, see this code from theme():

  // If a renderable array is passed as $variables, then set $variables to
  // the arguments expected by the theme function.
  if (isset($variables['#theme']) || isset($variables['#theme_wrappers'])) {
    $element = $variables;
    $variables = array();
    if (isset($info['variables'])) {
      foreach (array_keys($info['variables']) as $name) {
        if (isset($element["#$name"])) {
          $variables[$name] = $element["#$name"];
        }
      }
    }
    else {
      $variables[$info['render element']] = $element;
      // Give a hint to render engines to prevent infinite recursion.
      $variables[$info['render element']]['#render_children'] = TRUE;
    }
  }

However this should work:

  $image_resize_summary = array(
    '#theme' => 'image_resize_summary',
    '#data' => $variables['data'],
  );
  return drupal_render($image_resize_summary);
heddn’s picture

Status: Needs work » Needs review
FileSize
1019 bytes

Status: Needs review » Needs work
Issue tags: -CodeSprintUA

The last submitted patch, drupal-drupal_render_theme_image_crop_summary-2010122-17.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +CodeSprintUA
tsphethean’s picture

Status: Needs review » Reviewed & tested by the community

#17 looks good to me.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed aed78ff and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary: View changes

to test