The module should be based on Layout Discovery for 2.x branch

Can we test this out? i found the module couple of hours ago and thought i could help getting it to work with D8.3, seems to be working good with me right now.. BTW this module though probably needs version 2 instead of the current v1

So something is failing on D8.3 tests, Could someone help?

Updated the patch to resolve a bug that would not make region detection probably. I'm using the module with this patch and it's working great!

Thanks for your efforts @samerali. I'm currently unable to review your work due to time constraints. I'll get on this ASAP. Any reviews are most welcome in the mean time.

Seems to work as designed. I'm able to display the fields within a layout.

RTBC ++

Just a minor clean-up of description & both dependencies exists only in 8.3 core, so no reason to compare version

trying to fix test

Tests need fixing

Re-roll

trying to fix tests

- removed IntrospectableContainerInterface as SF 3.x has no such class (core 8.4 compatibility)
- replace deprecated Field views plugin with EntityField

Fix broken tests, probably 8.x-2.x better to use for new module's version

Clean-up coding standards regression

And one more minor fix

Commit please!

This still needs review before commiting, and probably new branch

This module is a MUST HAVE! This functionnality was include with Views on Drupal 7.
Does the update to the new branch will be easily to apply? Or does it break websites :/ ?
Thanks

is this confirmed working yet? can it be committed?

I'm using it but it needs maintainer to start new branch, I'd like file issue to ask co-maintainership

I currently don't have time for a proper functional review, but the patch itself looks good and I'd be happy to open a new branch and commit this if someone can RTBC this.

Reviewing at @andypost's request. 99% of it looks great!

1. +++ b/layout_plugin_views.info.yml
   @@ -1,7 +1,7 @@
   -description: 'Allows users to dispay Views fields in the regions of pluggable layouts provided trough the layout plugin module.'
   +description: 'Allows users to display Views fields in the regions of pluggable layouts provided trough the layout plugin module.'
   

   This changes unrelated typos, but leaves the incorrect part: no longer the layout plugin module, should be the Layout Discovery module

2. +++ b/src/RegionMap.php
   @@ -57,7 +57,7 @@ class RegionMap {
   -    return in_array($region_name, $this->getRegionNamesForSelectedLayout());
   +    return array_key_exists($region_name, $this->getRegionNamesForSelectedLayout());
   
   @@ -66,8 +66,9 @@ class RegionMap {
   -    $available_regions = array_keys($definition['region_names']);
   +    $available_regions = $definition->getRegionLabels();
   

   If the second part changed to using ->getRegionNames() instead, the first part wouldn't have to change.

3. +++ b/tests/src/Unit/FieldsTest.php
   @@ -3,23 +3,25 @@
   +use Drupal\Tests\UnitTestCase;
   ...
   -use Symfony\Component\DependencyInjection\IntrospectableContainerInterface;
   +use Symfony\Component\DependencyInjection\ContainerInterface;
   ...
   -class FieldsTest extends \PHPUnit_Framework_TestCase {
   +class FieldsTest extends UnitTestCase {
   
   @@ -28,7 +30,7 @@ class FieldsTest extends \PHPUnit_Framework_TestCase {
   -    $this->container = $this->getMockForAbstractClass(IntrospectableContainerInterface::class);
   +    $this->container = $this->getMockForAbstractClass(ContainerInterface::class);
   
   @@ -392,7 +394,7 @@ class FieldsTest extends \PHPUnit_Framework_TestCase {
   -    $field = $this->getMockBuilder(Field::class)
   +    $field = $this->getMockBuilder(EntityField::class)
   

   Out of scope

4. +++ b/tests/src/Unit/FieldsTest.php
   @@ -3,23 +3,25 @@
   -use Drupal\views\Plugin\views\field\Field;
   +use Drupal\views\Plugin\views\field\EntityField;
   

   Not sure about this

I did test of patch. On my side works ok https://i.gyazo.com/242217b83c0113befe5c5d2fdd4cc46d.png

@tim.plunkett Thanx for review, new patch fixes #26

1) also changed "provided trough/by"
2) also removed intermediate variable
3) reverted to PHPUnit_Framework_TestCase - it works with 8.6 core as well
4) this class deprecated in core since 8.3.x so IMO in scope

No reason to call method in loop as it was a variable

PHPUnit_Framework_TestCase is not supported on phpunit 6, which is what core uses for testing for php7.0 and up. Im not sure why the test isnt picking up on the fact that there is an alias for that class in core/tests/bootstrap.php, which is what makes most of the other ones work. Perhaps change that to '\PHPUnit\Framework\TestCase'

Not completely sure what the proper technique is for ensuring that the tests work on both phpunit 4.8 (for 5.5/5.6) and on phpunit 6 (for 7 and up), but thats probably where the issue lies.

Let's see if replace will help, otherwise will need to revert to UnitTestCase

Yep, this is how it works now but surely needs rewamp UI in separate issue when this one lands

I've finally found some time to catch up on maintenance of my modules so I've opened the 8.x-2.x branch for this. I'm willing to put some effort into reviewing this to make it land in the next couple of weeks.

Committed and pushed to 8.x-2.x. Thanks for your work on this everyone!

Great! Please update project page about difference between 2 and 1

Done!