Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helga.cheberakha’s picture

Assigned: Unassigned » helga.cheberakha

WDG (Ukraine,Kharkov) want to implement this on Code Sprint UA.

andypost’s picture

Issue tags: +CodeSprintUA

updated summary with related issues

helga.cheberakha’s picture

Status: Active » Needs review
Issue tags: -CodeSprintUA
FileSize
1.67 KB

Status: Needs review » Needs work
helga.cheberakha’s picture

Assigned: helga.cheberakha » Unassigned
Status: Needs work » Needs review
FileSize
1.67 KB

Status: Needs review » Needs work
somepal’s picture

Assigned: Unassigned » somepal
Status: Needs work » Needs review
FileSize
1.64 KB
1.64 KB

could not apply #5, throws fatal:corrupt patch at line 41.
re-rolling.

thedavidmeister’s picture

Status: Needs review » Needs work
Issue tags: +Novice
+        foreach ($theme->screenshot as $key => $value) {
+          $image['#' . $key] = $value;
+        }

Don't do this, explicitly set each variable that #theme 'image' is expecting. Please see the issue summary for the parent issue for more details.

+        $links = array(
+          '#theme' => 'links',
+          '#links' => $theme->operations,
+          '#attributes' => array(
+            'class' => array('operations', 'clearfix')
+          )
+        );

missing trailing commas in here.

jenlampton’s picture

Status: Needs work » Needs review
FileSize
1.81 KB

needed a reroll too, so...

azinoman’s picture

Assigned: somepal » azinoman
azinoman’s picture

Assigned: azinoman » Unassigned
Status: Needs review » Reviewed & tested by the community

I set the theme to the default and changed the theme a few times. I encountered no errors, everything looks good! Changing to reviewed and tested by the community.

star-szr’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/system/system.admin.incundefined
@@ -1281,7 +1281,19 @@ function theme_system_themes_page($variables) {
-      $screenshot = $theme->screenshot ? theme('image', $theme->screenshot) : '<div class="no-screenshot"><div class="no-screenshot__text">' . t('no screenshot') . '</div></div>';
...
+        $screenshot = '<div class="no-screenshot">' . t('no screenshot') . '</div>';

The markup is changing here but shouldn't - we are missing the no-screenshot__text div.

IshaDakota’s picture

Assigned: Unassigned » IshaDakota
IshaDakota’s picture

Assigned: IshaDakota » Unassigned
Status: Needs work » Needs review
FileSize
608 bytes
1.85 KB

Fixed markup change noted in #13.

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

The last submitted patch, twig-replace_themes_page-2010086-14.patch, failed testing.

IshaDakota’s picture

Status: Needs work » Needs review
Issue tags: +Novice
heddn’s picture

Status: Needs review » Needs work

Seem to be missing a call to theme('system_themes_page') in system.admin.inc @ line 221.

hussainweb’s picture

Status: Needs work » Needs review

@heddn, I think this issue only covers the theme() calls in theme_system_themes_page(). It seems all other calls are being sorted out in #2009674: Replace theme() with drupal_render() in system module.

hussainweb’s picture

Status: Needs review » Reviewed & tested by the community

BTW, the patch looks good and applies cleanly.

heddn’s picture

@hussainweb, good clarification. I agree this looks RTBC.

YesCT’s picture

Issue tags: +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 1a0f40d and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.