Page Manager's current method of providing variants for existing system paths involves overriding their Routes with Entity View routes. This is not ideal for at least two reasons:
- Information associated with original routes is lost
- The entire region setup as configured in the Panels variant content settings is rendered inside a 'Main Content' block.
I've made some changes that bypass Page Manager's route override entirely by the following process:
- A new
RouteSubscriber
class for Panels Everywhere removes any route overrides added by Page Manager's subscriber, and instead adds the page ID as a parameter in the original route's_defaults
. - Panels Everywhere's
DisplayVariantSubscriber
now checks theRouteMatch
for the new Page ID parameter and, if found, loads the corresponding Page entitty instead of the generic/site_template
one. The generic entity is still loaded for any routes that do not have a Page ID associated with them. - A checkbox for disabling route overrides is added to Page Manager's forms.
EDIT: the patch has since been committed to the main branch.
NOTE: currently, this requires page_manager to be patched with the following:
http://cgit.drupalcode.org/page_manager/commit/?id=292cddfeca17bea8b95fb...
This is the first patch I've ever submitted, the first issue I've ever opened, and the first time I've interacted on Drupal.org in any capacity - so I apologize in advance if any and all of the above fails to conform to the community's standards. Please let me know of anything I could have done better if you can - I would appreciate it very much!
Comments
Comment #2
nonAlgebraic CreditAttribution: nonAlgebraic commentedRe-roll with functioning access controls.
Background: currently,
PanelsEverywherePageDisplayVariantSubscriber
performs an access check on the page variant by calling the parentPageVariant
'sresolveConditions
function directly. This will always fail (for any variants with selection conditions) because the conditions returned bygetSelectionConditions
never undergo context mapping and thus when the execution gets to aContextAwarePlugin
there will not be a 'context' field in the configuration.Instead, the subscriber should make use of
PageVariant
's own access handler, which performs context mapping.Comment #3
nonAlgebraic CreditAttribution: nonAlgebraic commentedSome style improvements and a new overridable method in the variant subscriber class.
Comment #4
nonAlgebraic CreditAttribution: nonAlgebraic commentedAdded $event.stopPropagation() in the variant subscriber to ensure that if a matching page is found, no further page variant changes will occur (it is Panels everywhere after all, isn't it?)
Comment #5
nonAlgebraic CreditAttribution: nonAlgebraic commentedBetter decision logic between overriding routes and standalone ones.
Comment #6
DamienMcKennaComment #7
nonAlgebraic CreditAttribution: nonAlgebraic commentedComment #8
nonAlgebraic CreditAttribution: nonAlgebraic commentedCleaned up the variant resolution process.
Comment #9
nonAlgebraic CreditAttribution: nonAlgebraic commentedAdded a small helper function to get the main content from the variant, if available.
Comment #10
Alumei CreditAttribution: Alumei commentedIs this still relevant for the current 8.x-4.x branches of PE and this module?
Comment #11
nonAlgebraic CreditAttribution: nonAlgebraic commented@Alumei I'm running panels:4.2 and page_manager 4.0.0-beta2 (with core:8.4.0) and it's still relevant, as Page Manager still uses the "route override" strategy for existing routes, and is still subject to Panels' default approach of rendering the page variant as the "Main Content" block *within* the original page.
Comment #12
Alumei CreditAttribution: Alumei commented@nonAlgrebraic I tried running the tests on the 8.x-4.x branch, but the patch no longer applies, can you do a reroll of the patch against that branch?
Also, if I'm reading the commit-log of pange-manager right, the patch mentioned in the issue summary is already included in the 8.x-4.0-beta2 version. Should we maybe adjust the required page-manager version accordingly?
Comment #13
nonAlgebraic CreditAttribution: nonAlgebraic commentedComment #14
nonAlgebraic CreditAttribution: nonAlgebraic commented@Alumei sorry, I misread your previous comment. I understand now that the 8.x-4.x branch you're referring to is for Page Manager, not Panels Everywhere. The patches I've attached here are for Panels Everywhere.
You're also right about Page Manager merged the other patched I mentioned in the issue summary, so I edited the OP to reflect that it's no longer necessary. Thanks!
Comment #15
Alumei CreditAttribution: Alumei commented@nonAlgrebaic Oh, I see. Some additional info might have been helpful.
Actually I meant the 8.x-4.x of Panels Everywhere. From the issue queue I gathered that development for the 1.x was dropped in favor of creating 4.x release, matching panels and page managers versions.
Therefore I asked about the applicability for the 8.x-4.x branch in my comment #10. After your remark I triggered the Testbot to apply your latest patch to the 4.x branch and run the recently added tests.
There the Testbot informed that the patch does not apply, see the results for you patch in #9.
As such I wanted to ask you in #12 to reroll the patch so that it applies to the 8.x-4.x branch.
I hope this helps, sorry for the misunderstanding.
Comment #16
Alumei CreditAttribution: Alumei commentedAttempted reroll of the patch in #9 against 8.x-4.x branch
Comment #18
nonAlgebraic CreditAttribution: nonAlgebraic commentedComment #19
nonAlgebraic CreditAttribution: nonAlgebraic commented@alumei gotcha, sorry for the miscommunication!
Here's a re-roll that should work.
Comment #20
nonAlgebraic CreditAttribution: nonAlgebraic commentedOops, bad upload. Here we go.
Comment #21
Alumei CreditAttribution: Alumei commentedNote: I just created #2933423: Content of placed title block does not show up and remembered seeing a fix for that in this issue #9. I'll try applying it over there.
Comment #22
Alumei CreditAttribution: Alumei commentedFixed the test failures.
This will need additional tests as the patch adds at least these new features:
Comment #23
DamienMcKennaNeeds work per #22.
Comment #24
Alumei CreditAttribution: Alumei commentedI added a unit test for
PanelsEverywhereRouteSubscriber
and fixed some edge cases in the process.Needs review only to trigger testbot.
Comment #25
DamienMcKennaComment #27
DamienMcKennaCommitted. Thanks.