Similar to #2035697: Entity reference formatter should default to the 'Default' view mode.

#2035697: Entity reference formatter should default to the 'Default' view mode
#2057203: Unable to select the 'Default' view mode in views entity area handler
#2057207: Unable to select the 'Default' view mode in views entity row plugin

CommentFileSizeAuthor
#14 drupal-2057199-14-fail.patch1.16 KBolli
#14 drupal-2057199-14-pass.patch1.98 KBolli
#10 drupal-2057199-10-complete.patch2.35 KBlauriii
#5 drupal-2057199-5-tests.patch1.95 KBlauriii
#1 drupal-2057199-1.patch830 bytesolli
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

olli’s picture

olli CreditAttribution: olli commented
Status: Active » Needs review
FileSize
drupal-2057199-1.patch830 bytes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Thanks
Should we expand tests here?

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Tagging, I think tests would be good here.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented

So we need to create a test for this

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
FileSize
drupal-2057199-5-tests.patch1.95 KB

Here's a test for this.

swentel’s picture

swentel CreditAttribution: swentel commented
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

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented

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

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

@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

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Needs work » Needs review
FileSize
drupal-2057199-10-complete.patch2.35 KB

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

olli CreditAttribution: olli commented

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

marthinal CreditAttribution: marthinal commented

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

marthinal’s picture

marthinal CreditAttribution: marthinal commented
Issue summary: View changes

Updated issue summary.

olli’s picture

olli CreditAttribution: olli commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
drupal-2057199-14-pass.patch1.98 KB
drupal-2057199-14-fail.patch1.16 KB

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

olli CreditAttribution: olli commented
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

Ready to fly

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
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

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

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

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

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.