Page Manager provides a nice Context plugin that doesn't load the entity until it is needed. This would be good to bring into CTools, especially as the ContextManager being worked on in #2511568: Create "context stack" service where available contexts can be registered is hopefully moving the StaticContext event listener into ctools.

CommentFileSizeAuthor
#14 2571699-14.patch448 bytesandypost
#2 2571699-2.patch1.9 KBrlmumford

Comments

rlmumford created an issue. See original summary.

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented
Status: Active » Needs review
StatusFileSize
new 2571699-2.patch1.9 KB

Here is a patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2571699-2.patch, failed testing.

Status: Needs work » Needs review

rlmumford queued 2: 2571699-2.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2571699-2.patch, failed testing.

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented

I have no idea how this would kill the Wizard tests.

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented
Issue tags: +D8panels

Adding to panels sprint board.

Status: Needs work » Needs review

rlmumford queued 2: 2571699-2.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2571699-2.patch, failed testing.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Some changes were made to this code in Page Manager for recent core changes, see #2576323: Fix static context for recent core changes

Status: Needs work » Needs review

rlmumford queued 2: 2571699-2.patch for re-testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +Needs tests

Tests would be great.

eclipsegc’s picture

eclipsegc commented
Status: Needs review » Needs work

Double check against page_manager's implementation and add some test coverage. I'd love to see this in ctools.

Eclipse

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 2571699-14.patch448 bytes

There's identical implementation of the class and tests in both modules \Drupal\Tests\ctools\Unit\ContextMapperTest::testGetContextValuesEntityContext

Only class description missing

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
commented
Status: Needs review » Fixed

This was fixed in commit 3ebc76216d168d91862234bdde1c62b4389b7168

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Fixed » Reviewed & tested by the community

My patch still applies and fix documentation issue

  • joelpittet committed fc34e36 on 8.x-3.x authored by andypost 
    Issue #2571699 by rlmumford, andypost: Include EntityLazyLoadContext...
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Reviewed & tested by the community » Fixed

Closing this out, thanks @andypost

Status: Fixed » Closed (fixed)

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