Allowing content type plugins to specify a weight makes it possible to render their panes before or after other panes.

An example of when this is useful is a module I'm working on called Solr Panels. Various Apache Solr blocks expect the search to have been executed in the menu callback, but because Solr Panels executes the search when rendering the Results pane, any panes being rendered before that won't work. If I could set the Results pane to render first, all would be good :)

This patch adds support for a weight property in the $plugin array of content types. It still respects the "render last" property, which will still render last.

Comments

merlinofchaos’s picture

I toyed with weighting, but generally found a single phase of 'render last' was sufficient, and weighting adds time to the render that I'm not comfortable with. Are you 100% sure that render last is not sufficient for this?

ximo’s picture

StatusFileSize
new2.39 KB

Unfortunately, yes. The thing is I can only set "render last" on content type plugins that I create, which excludes any Apache Solr panes from other modules that may be rendered before mine. We tried exposing other Apache Solr modules' blocks through our own content type based on "block" and setting "render last" on those, but can't possibly account for all modules interacting with Apache Solr.

I understand your point on rendering time. How about a "render first" property then? I don't think fine-tuning of weight is needed, and as long as my pane is rendered "firstish", I'm good.

Something like this patch?

merlinofchaos’s picture

Going to have to give some thought. That ends up being a three phase weighting, which is probably still faster to process than a truly weighted system, but may ultimately end up inferior due to code complexity. That said, removing render last is not an option at this point, so it seems like that is the better way to go. I'll give it some thought, but render first does appeal a little bit, but I want to let it rattle around in my head a bit before I come to a conclusion. Might be nice to also get some other opinions here.

ximo’s picture

Title: Add weighting to content type plugins » Add "render first" to content type plugins
StatusFileSize
new2.35 KB

The "render first" property seems a logical addition when there's already a "render last". And I can't see how it would affect the render time much. I've tried to alleviate the code complexity though, check out the patch. I've asked sdboyer if he could give his opinion as well.

sdboyer’s picture

Since I've been refactoring the rendering pipeline of late, this question has been on my mind quite a bit. The 'render first' feels to me like a reasonable step in our current direction. I'd not be opposed to full-blown weighting, though - I don't think the cost would be that bad, as the number of items being sorted will be limited to the actual panes to be rendered - never a super-high number.

Doesn't really fix the the underlying problem, though: we've got stuff that needs to render before or after some other stuff. Which basically means we need a directed acyclic graph to describe those things, and determine the proper render order. In the short term, weights can work (especially if we set some standards for weighting - e.g., page elements base weighting is 20, Solr is -20, etc.), but I think we're going to need to revisit this eventually.

merlinofchaos’s picture

Status: Needs review » Needs work

Hmm. This patch needs to be refactored for the new render pipeline I think. I might give a stab at this later if you're busy, ximo.

merlinofchaos’s picture

Status: Needs work » Needs review
StatusFileSize
new3.93 KB

Here's a reroll for the new render pipeline. The logic is significantly simpler in this version.

merlinofchaos’s picture

I realize 'weight' would be valuable (and it probalby wouldn't be that bad) but it would be difficult to integrate it with the existing 'render last' functionality, I think. But that is a possibility to consider, I guess.

ximo’s picture

Thanks for the reroll! Looks nice. I'll give it a spin later today.

I've been very busy lately, but I'd like to work on weighting during the code sprint in DrupalCon CPH.

ximo’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new4.69 KB

Alright, I've finally had a look at your patch and it looks very good! There's just one small typo, a trailing space in $content_type['render first ']. Fixed the typo and added a comment for the elseif. Have tested the patch with Apache Solr Panels (my module), and everything works as expected. RTBC as far as I'm concerned!

A complete weighting system can always be worked in later – adding 'render first' shouldn't complicate that process any more than 'render last' might.

merlinofchaos’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks for the re-review!

Status: Fixed » Closed (fixed)

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

  • Commit 77a6bb5 on 7.x-3.x, master, pipelines, 7.x-3.x-i18n, 8.x-3.x by sdboyer:
    #806874 by ximo: Allow panes to specify "render first" as well as "...