Skip to content
Snippets Groups Projects

Add an Include image in display checkbox to the image field

Open Andrei Ivnitskii requested to merge issue/drupal-2040033:2040033-add-an-include into 11.x
5 unresolved threads

Closes #2040033

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
631 $node = $this->drupalCreateNode(['type' => $type_name]);
632 // Check image last as the assertions below assume that this is the case.
633 $image_formatters = ['hidden', 'image'];
634 foreach ($image_formatters as $formatter) {
635 if ($formatter === 'hidden') {
636 $edit = [
637 "fields[$field_name][region]" => 'hidden',
638 ];
639 }
640 else {
641 $edit = [
642 "fields[$field_name][type]" => $formatter,
643 "fields[$field_name][region]" => 'content',
644 ];
645 }
646 $this->drupalGet("admin/structure/types/manage/{$type_name}/display");
  • Comment on lines +635 to +646

    Can we do this using ->setDisplay on the entity view display, we don't need to submit the form to make this change - we already have test coverage for the manage display form, so for performance sake should focus on what we're testing here - which is the display setting

  • Please register or sign in to reply
  • 646 $this->drupalGet("admin/structure/types/manage/{$type_name}/display");
    647 $this->submitForm($edit, 'Save');
    648 $this->drupalGet('node/' . $node->id());
    649 // Verify that the field label is hidden when no image is attached.
    650 $this->assertSession()->pageTextNotContains($field_name);
    651 }
    652
    653 $test_image = current($this->drupalGetTestFiles('image'));
    654
    655 // Create a new node with the uploaded image.
    656 $nid = $this->uploadNodeImage($test_image, $field_name, $type_name, 'image');
    657
    658 // Check that the default formatter is displaying with the image name.
    659 $node_storage = $this->container->get('entity_type.manager')->getStorage('node');
    660 $node_storage->resetCache([$nid]);
    661 $node = $node_storage->load($nid);
    • Comment on lines +659 to +661

      We can do this in one pass with ::loadUnchanged

      Suggested change
      659 $node_storage = $this->container->get('entity_type.manager')->getStorage('node');
      660 $node_storage->resetCache([$nid]);
      661 $node = $node_storage->load($nid);
      659 $node_storage = $this->container->get('entity_type.manager')->getStorage('node');
      660 $node = $node_storage->loadUnchanged($nid);
    • Please register or sign in to reply
  • 672 $this->assertSession()->responseContains($default_output);
    673
    674 // Turn the "display" option off and check that the image is no longer displayed.
    675 $edit = [$field_name . '[0][display]' => FALSE];
    676 $this->drupalGet('node/' . $nid . '/edit');
    677 $this->submitForm($edit, 'Save');
    678
    679 $this->assertSession()->responseNotContains($default_output);
    680
    681 // Uncheck the display checkboxes and go to the preview.
    682 $this->drupalGet("node/{$nid}/edit");
    683 $edit[$field_name . '[0][display]'] = FALSE;
    684 $this->submitForm($edit, 'Preview');
    685 $this->clickLink('Back to content editing');
    686 $this->assertSession()->responseContains($field_name . '[0][display]');
    687
  • 664 '#theme' => 'image',
    665 '#uri' => $file->getFileUri(),
    666 '#width' => 40,
    667 '#height' => 20,
    668 '#alt' => 'image',
    669 '#attributes' => ['loading' => 'lazy'],
    670 ];
    671 $default_output = str_replace("\n", '', (string) \Drupal::service('renderer')->renderRoot($image));
    672 $this->assertSession()->responseContains($default_output);
    673
    674 // Turn the "display" option off and check that the image is no longer displayed.
    675 $edit = [$field_name . '[0][display]' => FALSE];
    676 $this->drupalGet('node/' . $nid . '/edit');
    677 $this->submitForm($edit, 'Save');
    678
    679 $this->assertSession()->responseNotContains($default_output);
    • we also need a positive assert here, this would pass if the page was a WSOD too. Something like asserting the response code is a 200. OR perhaps if we had two images that the second one was shown whilst the first one was not.

    • Please register or sign in to reply
  • 657
    658 // Check that the default formatter is displaying with the image name.
    659 $node_storage = $this->container->get('entity_type.manager')->getStorage('node');
    660 $node_storage->resetCache([$nid]);
    661 $node = $node_storage->load($nid);
    662 $file = $node->{$field_name}->entity;
    663 $image = [
    664 '#theme' => 'image',
    665 '#uri' => $file->getFileUri(),
    666 '#width' => 40,
    667 '#height' => 20,
    668 '#alt' => 'image',
    669 '#attributes' => ['loading' => 'lazy'],
    670 ];
    671 $default_output = str_replace("\n", '', (string) \Drupal::service('renderer')->renderRoot($image));
    672 $this->assertSession()->responseContains($default_output);
    • Comment on lines +663 to +672

      this is a pretty tight coupling to the internals of the image formatter. Could we simplify this down to something that checked an img element with the expected url exists - rather than asserting the complete output from the theme hook?

    • Please register or sign in to reply
  • Like longwave, I'm not sure there is a need for this feature. But that said, here's a review - mostly focused on the test

  • Please register or sign in to reply
    Loading