Problem/Motivation

BlockDisplayVariant has logic for handling selection criteria, but this is the domain of page_manager's page_variant entity, not variant display plugins.

Proposed resolution

Delete the properties and defaults and update tests where applicable.

Remaining tasks

TODO

User interface changes

TODO

API changes

TODO

Data model changes

TODO

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

EclipseGc created an issue. See original summary.

EclipseGc’s picture

Status: Active » Needs review
FileSize
6.34 KB

Ok, this removes some tests since our access method is now core's which already has test coverage, and generally cleans up stuff that was using this interface.

CTools, Page Manager and Layout Plugin tests are all passing with the exception of an error in layout_plugin for an unrelated ctools change.

Eclipse

dsnopek’s picture

Walked through all these changes with EclipseGC and witnessed the tests passing. :-)

I'm +1 on this, but I'll leave it to @tim.plunkett to RTBC!

tim.plunkett’s picture

+++ b/src/Plugin/ConditionVariantInterface.php
@@ -11,6 +11,10 @@ use Drupal\Core\Display\VariantInterface;
+ * @deprecated
+ *   This interface was intended for use with VariantInterface::access but that
+ *   method should not exists in core so neither should this interface.
  */
 interface ConditionVariantInterface extends VariantInterface {

You need to say what version it will be removed in.

But that said, why not just remove it instead of waiting? We're in alpha after all...

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
6.68 KB
11.99 KB

And honestly, you can just remove ConditionVariantTrait outright too. Sure, page_manager is using it, but wrongly. And I can easily fix it (page_manager will be broken after the CTools release anyway)

dsnopek’s picture

Removing the trait sounds great to me! RTBC +1 :-)

EclipseGc’s picture

Status: Reviewed & tested by the community » Fixed

David and I discussed tim's additional removals here and decided while ctools would ideally provide a trait and interface for this sort of behavior, theres are named wrong at this point, and the trait is inconsistent in how to does various things, so perhaps we'll try again later with something that's not variant specific.

Eclipse

Status: Fixed » Closed (fixed)

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