Attachments currently are not able to be added to Embed displays. It seems like this should be allowed.

Use case

From #10

It's been a while, haha, but I think the use case was needing to embed a view on a landing page content type using a field to select the View / Display. The embed display used an offset to exclude the first item, and an attachment was used to prefix the view with the first item in a different, more pronounced, style.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

blake.thompson created an issue. See original summary.

blake.thompson’s picture

Version: 8.3.x-dev » 8.4.x-dev
blake.thompson’s picture

Attached patch that allows attachments to be added to Embed displays.

dawehner’s picture

That totally makes sense. @blake.thompson Do you think it makes sense to add a test here?

blake.thompson’s picture

I don't think this particular change needs a test, it's just making use of the DisplayPluginBase boolean like Block and Page. If there should be more tests around the variable or method that uses it, then perhaps a test(s) should be added.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.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.5.x-dev » 8.6.x-dev

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

blake.thompson’s picture

Updating patch with {@inheritdoc}

dawehner’s picture

Category: Feature request » Task

I don't see a reason not to support it. To be honest this is more of a task than a feature :)

One thing which would be nice, as it helps the committers: Name a concrete usecase you need this for.

blake.thompson’s picture

Issue summary: View changes

It's been a while, haha, but I think the use case was needing to embed a view on a landing page content type using a field to select the View / Display. The embed display used an offset to exclude the first item, and an attachment was used to prefix the view with the first item in a different, more pronounced, style.

dawehner’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Thank you @blake.thompson, I copied your statement into the issue summary.

I don't think we need tests given that we have this boolean flag tested in all the attachment tests already.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This seems sensible. I tried it out and looked through the code for anything that might break. Couldn't spot anything. I contemplated whether or not this needs a change record and I don't think so.

Committed 5550b76 and pushed to 8.6.x. Thanks!

  • alexpott committed 5550b76 on 8.6.x
    Issue #2886613 by blake.thompson: Allow Attachments on Embed Displays
    

Status: Fixed » Closed (fixed)

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

jastraat’s picture

Has anyone actually tested this? While attachments can now be linked to an embed display, if you try to display a summary in the attachment (as though you were going to create a glossary-like view+attachment) - it fails silently with no view results.