Comments

olli’s picture

Status:Active» Needs review
StatusFileSize
new830 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,919 pass(es).
[ View ]
larowlan’s picture

Thanks
Should we expand tests here?

benjy’s picture

Status:Needs review» Needs work
Issue tags:+Needs tests

Tagging, I think tests would be good here.

lauriii’s picture

So we need to create a test for this

lauriii’s picture

StatusFileSize
new1.95 KB
FAILED: [[SimpleTest]]: [MySQL] 58,802 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here's a test for this.

swentel’s picture

Status:Needs work» Needs review

The test will most likely fail because of #2099417: Unable to change Custom Block view mode - I'm wondering though if it makes sense at all to expose 'Default'. We don't allow this anywhere else with view modes.

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockCreationTest.php
@@ -58,6 +58,13 @@ public function testCustomBlockCreation() {
+    // Change view mode to default. ¶

Space at the end which should go.

lauriii’s picture

We should run the tests with the patch swentel have applied to #2099417: Unable to change Custom Block view mode

Status:Needs review» Needs work

The last submitted patch, drupal-2057199-5-tests.patch, failed testing.

Dave Reid’s picture

@swentel: See the issue summary. The default view mode is exposed in entity reference and in Views. It was wrong of us *not* to expose it since it's exposed in the Field UI.

lauriii’s picture

Status:Needs work» Needs review
StatusFileSize
new2.35 KB
FAILED: [[SimpleTest]]: [MySQL] 58,425 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here's a patch#5 and swentel's patch in #2099417: Unable to change Custom Block view mode together.

Status:Needs review» Needs work

The last submitted patch, drupal-2057199-10-complete.patch, failed testing.

olli’s picture

Nice work!

  1. +++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Plugin/Block/CustomBlockBlock.php
    @@ -113,6 +113,7 @@ public function blockForm($form, &$form_state) {
       public function blockSubmit($form, &$form_state) {
         // Invalidate the block cache to update custom block-based derivatives.
         if ($this->moduleHandler->moduleExists('block')) {
    +      $this->configuration['view_mode'] = $form_state['values']['custom_block']['view_mode'];

    Is there a reason why this has to go inside the if?

  2. +++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockCreationTest.php
    @@ -58,6 +58,13 @@ public function testCustomBlockCreation() {
    +    // Check that the view mode has actually changed to default.
    +    $this->clickLink($edit['info']);

    We should click the "Configure" link instead.

marthinal’s picture

Patch attached with a possible test that works here #2099417: Unable to change Custom Block view mode

marthinal’s picture

Issue summary:View changes

Updated issue summary.

olli’s picture

Issue summary:View changes
Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new1.98 KB
PASSED: [[SimpleTest]]: [MySQL] 58,061 pass(es).
[ View ]
new1.16 KB
FAILED: [[SimpleTest]]: [MySQL] 57,987 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

The other issue went in. Here's a small test for available view mode options.

Status:Needs review» Needs work

The last submitted patch, 14: drupal-2057199-14-fail.patch, failed testing.

olli’s picture

Status:Needs work» Needs review
larowlan’s picture

Status:Needs review» Reviewed & tested by the community

Ready to fly

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x, although I'm concerned given how many other modules have had this problem given the xrefs in the issue summary that this may be the wrong way to fix it. Can default be added after the fact somewhere in field UI if it doesn't already exist? (e.g. $options += array('default' => 'Default')?)

webchick’s picture

Status:Reviewed & tested by the community» Fixed

...let's try that again, drupal.org.

Dave Reid’s picture

It's not a matter of adding it in the Field UI. It's a matter of all these little places that expose the view modes in some kind of selection. It would be nice if we had some kind of helper function to retrieve a list of view modes for a select box array, that automatically added the 'default' one (and possibly didn't allow view modes that haven't been 'configured' yet to be selected). I'll file an issue for that.

Status:Fixed» Closed (fixed)

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