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

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

Status: Active » Needs review
StatusFileSize
new586 bytes
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
StatusFileSize
new585 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
StatusFileSize
new647 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
StatusFileSize
new663 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
StatusFileSize
new1016 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
StatusFileSize
new1019 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