Problem/Motivation

Region.html.twig is adding yet another layer of templates, div & classes to Drupal, thats only usable in very specific cases.

Proposed resolution

Remove the default wrapper div & classes from region.html.twig, so its still usable for a theme that need that kind of flexibility.

region.html.twig

{% if content %}
    {{ content }}
{% endif %}

Remaining tasks

test bartik
test seven
write test
wrap markup classes out into the template

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#1 drupal-2226953-remove-wrapper-1.patch357 bytesmandar.harkare
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,790 pass(es), 8 fail(s), and 2 exception(s). View

Comments

mandar.harkare’s picture

Status: Active » Needs review
FileSize
357 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,790 pass(es), 8 fail(s), and 2 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 1: drupal-2226953-remove-wrapper-1.patch, failed testing.

botanic_spark’s picture

Assigned: Unassigned » botanic_spark
botanic_spark’s picture

I checked the tests that are failed and started from Drupal\block\Tests\BlockTest.
I installed latest clean Drupal installation and when i tried to run that test I encountered an error. The test was broken.

The error was
Fatal error: Call to a member function label() on a non-object in /home/www/ddd/www/core/modules/block/lib/Drupal/block/Tests/BlockTest.php on line 116

The problematic part of code is

function testBlock() {
    // Select the 'Powered by Drupal' block to be configured and moved.
    $block = array();
    $block['id'] = 'system_powered_by_block';
    $block['settings[label]'] = $this->randomName(8);
    $block['theme'] = \Drupal::config('system.theme')->get('default');
    $block['region'] = 'header';

    // Set block title to confirm that interface works and override any custom titles.
    $this->drupalPostForm('admin/structure/block/add/' . $block['id'] . '/' . $block['theme'], array('settings[label]' => $block['settings[label]'], 'id' => $block['id'], 'region' => $block['region']), t('Save block'));
    $this->assertText(t('The block configuration has been saved.'), 'Block title set.');
    // Check to see if the block was created by checking its configuration.
    $instance = entity_load('block', $block['id']);

    $this->assertEqual($instance->label(), $block['settings[label]'], 'Stored block title found.');

When debugging i found out that $instance = entity_load('block', $block['id']); is returning NULL and after that it crashes.

With further inspection i found out that problem is in line

$this->drupalPostForm('admin/structure/block/add/' . $block['id'] . '/' . $block['theme'], array('settings[label]' => $block['settings[label]'], 'id' => $block['id'], 'region' => $block['region']), t('Save block'));

and the block is not created. The headers returned are either 404 or 401.

I was stuck here and couldn't continue further.

Can anyone try to reproduce this and give some suggestions?

nicholasruunu’s picture

nicholasruunu’s picture

I can reproduce it, but I'm getting the error even without applying the patch.

nicholasruunu’s picture

This is what I had to do to get the tests showing correctly without the weird error:
sudo -u _www php core/scripts/run-tests.sh --verbose --url http://drupal8.dev --file core/modules/block/lib/Drupal/block/Tests/BlockTest.php

I also did:
chmod -R 777 sites/default/files/simpletest sites/simpletest
chown -R _www:_www sites/default/files/simpletest sites/simpletest

Starting to look into the real issues now.
I think some block tests are using those arguments for text assertions.

nicholasruunu’s picture

Yeah, the problem is that some test is using those attributes in structure/blocks, so the tests have to be rewritten somehow:
core/modules/block/lib/Drupal/block/Tests/BlockTest.php:261

    // Confirm that the custom block was found at the proper region.
    $xpath = $this->buildXPathQuery('//div[@class=:region-class]//div[@id=:block-id]/*', array(
      ':region-class' => 'region region-' . drupal_html_class($region),
      ':block-id' => 'block-' . str_replace('_', '-', strtolower($block['id'])),
    ));
    $this->assertFieldByXPath($xpath, NULL, t('Block found in %region_name region.', array('%region_name' => drupal_html_class($region))));
nicholasruunu’s picture

Assigned: nicholasruunu » Unassigned

-- removed old comment

LewisNyman’s picture

Issue tags: +frontend, +html
gnuget’s picture

These tests make sure the blocks are capable to be moved between regions, now the problem is with this change we remove the reference in which region the block is positioned.

This comment is just for pointing exactly what the problem is.

lauriii’s picture

Assigned: Unassigned » lauriii
lauriii’s picture

I discussed about this with Joel and we found following problems when implementing this:

  • Demo mode - How do we show the regions in the Block modules region demonstration? We cannot demonstrate regions without region wrappers.
  • Quick edit mode - I'm not sure what this part of code does but this patch requires region wrapper.
  • Tests - We have tests that are testing regions etc. We need some alternative solution for testing these.
  • Bartik - Here we have to override the template. No problem but just a reminder.
lauriii’s picture

Assigned: lauriii » Unassigned
lauriii’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Postponed

I don't think this fits in anymore.

lauriii’s picture

Component: theme system » Classy theme

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.