Allow passing contexts to display variants (to enable Panels Everywhere to use static context and relationships)

1. +++ b/core/lib/Drupal/Core/Display/ContextAwareVariantInterface.php
   @@ -0,0 +1,33 @@
   +interface ContextAwareVariantInterface {
   

   should this extend the existing PageVariantInterface?

2. +++ b/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php
   @@ -204,6 +205,9 @@ protected function prepare(array $main_content, Request $request, RouteMatchInte
   +      if ($page_display instanceof ContextAwareVariantInterface) {
   

   <preference class="personal">personally I feel use of instanceof is an anti-pattern that makes the caller to aware of the details of the items it is dealing with, a kind of hidden dependency - can we add these methods to PageVariantInterface and provide a base-class that implements a null op?</preference>

3. +++ b/core/lib/Drupal/Core/Render/PageDisplayVariantSelectionEvent.php
   @@ -33,6 +33,13 @@ class PageDisplayVariantSelectionEvent extends Event {
   +  protected $contexts = [];
   
   @@ -92,6 +99,29 @@ public function setPluginConfiguration(array $configuration) {
   +  public function getContexts() {
   ...
   +  public function setContexts(array $contexts) {
   

   If we made the trait generic, we could use it here too.

4. +++ b/core/modules/system/tests/modules/display_variant_test/src/Plugin/DisplayVariant/TestDisplayVariant.php
   @@ -18,7 +20,9 @@
   +class TestDisplayVariant extends VariantBase implements PageVariantInterface, ContextAwareVariantInterface {
   

   If the later interface extends the former, we can simplify this

3. the methods|properties you add to this object exist in the trait, can we re-use the trait instead?

2. yep, I'm just registering my objection to this pattern in core, the horse has bolted on that

1. right, sorry about that, wrong interface - but there is one we can extend, sweet

4. agree

So just the question as to whether we can use the new trait instead of those methods|properties.

+++ b/core/lib/Drupal/Core/Display/ContextAwareVariantTrait.php
@@ -0,0 +1,51 @@
+ * Contains \Drupal\Core\Plugin\Display\ContextAwareVariantTrait.
...
+trait ContextAwareVariantTrait {

Sorry to be a pain, but perhaps this should be Drupal\Core\Plugin\Context\ContextAware trait as it has re-use potentional beyond variants in my opinion

Other than that (and I rarely weigh in on naming, so sorry) - I think this is ready.

Might be worth pinging Tim Plunkett for another set of eyes.

Good point @larowlan

This patch looks quite solid and works as described.

+++ b/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php
@@ -236,6 +237,9 @@ protected function prepare(array $main_content, Request $request, RouteMatchInte
+      if ($page_display instanceof ContextAwareVariantInterface) {
+        $page_display->setContexts($event->getContexts());
+      }

+++ b/core/lib/Drupal/Core/Render/PageDisplayVariantSelectionEvent.php
@@ -18,6 +19,8 @@
 class PageDisplayVariantSelectionEvent extends Event {
 
+  use ContextAwareTrait;
+

These are huge changes. It feels very, very strange to make this event context-aware, and to require the HTML renderer to deal with this also.

I just want to ask the question: Is this really necessary?

I think the answer is yes, and so in that case I'd like to ask you to update the docblocks of HtmlRenderer and PageDisplayVariantSelectionEvent to contain essentially the explanation in the IS.

Comment changes look good, I would agree with dsnopek that the comment for HtmlRenderer doesn't belong in the docblock but it is fine to leave it as code comment.

+++ b/core/lib/Drupal/Core/Display/ContextAwareTrait.php
@@ -0,0 +1,47 @@
+ * Contains \Drupal\Core\Plugin\Display\ContextAwareTrait.

If we're making this super generic, it's in the wrong namespace. What about just moving it up a level, out of Display ?

Actually, once I applied the patch and looked at it more, and looked at how page_manager would look after adapting to this, we should NOT make a weird generic trait just to save on copy/paste.

The trait should be there to satisfy an interface, but one of the two uses are the Event, which doesn't use that interface.

Also, other than the test there will likely just be one other usage of the trait anywhere...

So we don't need it!

Plus, then we get to tailor the docs on the methods on the Event to the actual use case.

Was just about to raise the same concert about having this generic ContextAware trait, but in regards to name confusion with ContextAwarePluginBase which might easily could be mistaken for something that used the ContextAware trait for plugins.

+1 for this direction, code looks good and coverage of it seems sufficient

The doc blocks were backwards on the trait (they were {@inheritdoc} in page_manager), this swaps them :)

+5. thanks for fixing that switched docblock.

1. +++ b/core/lib/Drupal/Core/Display/ContextAwareVariantInterface.php
   @@ -0,0 +1,33 @@
   + * Contains \Drupal\Core\Plugin\Display\ContextAwareVariantInterface.
   + */
   +
   +namespace Drupal\Core\Display;
   

   Wrong namespace in @file doc.

2. +++ b/core/lib/Drupal/Core/Display/ContextAwareVariantInterface.php
   @@ -0,0 +1,33 @@
   +interface ContextAwareVariantInterface extends VariantInterface {
   

   I feel like this should be just a ContextAwareInterface in Drupal\Core\Plugin\ but... I now know Tim isn't a fan of that. Not going to hold this up on this because that admittedly could open a can of worms.

3. +++ b/core/lib/Drupal/Core/Render/PageDisplayVariantSelectionEvent.php
   @@ -13,6 +13,10 @@
   + * It also allows you to pass along any additional information that the variant
   + * needs to work, for example: configuration, and any contexts (with values)
   + * that can't be found globally.
   + *
   

   Outside of this patch, how do I do pass along that information? I think an interface would make this clearer but we could just make the docs clearer maybe.

   "Context methods allow additional configuration and values to be passed that can't be found globally." ?

1) Yep.

2) We have \Drupal\Core\Plugin\ContextAwarePluginInterface, \Drupal\Component\Plugin\ContextAwarePluginInterface, \Drupal\Core\Plugin\Context\ContextAwarePluginManagerInterface, and even \Symfony\Component\Routing\RequestContextAwareInterface.
Now we're adding \Drupal\Core\Display\ContextAwareVariantInterface.

The term "ContextAware" is overloaded already. How would adding ContextAwareInterface help remove confusion?

3) Came up with this in IRC after further discussion.

2) Because the ContextAwarePluginInterface should be small enough we could use it here. Too late to split it up into composable interfaces though I guess.

Thanks!

+++ b/core/lib/Drupal/Core/Render/PageDisplayVariantSelectionEvent.php
@@ -13,6 +13,11 @@
+ * Subscribers of this event can call the following setters to pass additional

This should be overwrite, based on the setContexts function which overwrites the context array with the passed in array of contexts.

That's an implementation detail. We have setters like this all over, if you care about existing values you can do:

$event->setContexts($my_contexts + $event->getContexts());

Or similar.

Patch looks nice & simple.

+++ b/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php
@@ -236,6 +237,14 @@ protected function prepare(array $main_content, Request $request, RouteMatchInte
+      // context (ex. a context that refers to the Node with nid 6), If any

Nit: I've never seen "ex." being used before.

s/If/if/

Contributed project blocker, got walked through the patch at DrupalCon. Woot.

Committed and pushed to 8.0.x. Thanks!