Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
We are using Fieldable Panels Panes to create fieldable entities for use in panels. While we are primarily placing these panes in panel pages, it would be useful to be able to also occasionally place them outside of panels without having to use the Bean module, which would duplicate configuration and data (see: #1860118: Comparison with the Bean module).
I've created a patch that adds a checkbox to expose select FPPs as blocks for use outside of panels. Please note that this patch adds a new database column, so update.php must be run after applying it.
Comments
Comment #1
jlapp CreditAttribution: jlapp commentedAttaching patch.
Comment #2
Dave ReidWondering why we can't just re-use the existing "Make this entity reusable" field? That would make sense to me. I want this item to be able to be placed in Panels and/or Blocks.
Comment #3
jlapp CreditAttribution: jlapp commentedDave, thanks for the quick feedback. We certainly could do that. I was thinking you might have wanted to keep them separate, but simply reusing the existing field makes the patch much smaller and removes the need for a schema change. I'm attaching an updated patch that works as you described. Please let me know if you have any other feedback or suggestions!
Comment #4
asanchez75 CreditAttribution: asanchez75 commented#3: fieldable_panels_panes-render_as_block-1910934-3.patch queued for re-testing.
Comment #4.0
asanchez75 CreditAttribution: asanchez75 commentedUpdate issue description to reflect latest patch implementation
Comment #5
thrnio CreditAttribution: thrnio commentedThe patch in #3 adds a dependency to Entity.
Comment #6
thrnio CreditAttribution: thrnio commentedHere's a quick-and-dirty updated patch to #3 that just adds a dependency for Entity. Should probably evaluate whether this really warrants adding Entity as a dependency.
Comment #7
Pere OrgaI disagree with #2 about reusing the "Make this entity reusable" field. You may want to have many reusable panes without having to create lots of blocks. That is the current behaviour.
IMO It makes sense to keep that as a separate field like in patch #1. If it is not accepted the functionality could be moved to another contrib module (and in that case, reusing that checkbox like in patch #7 could be ok).
Comment #8
DamienMcKennaI think it'd be useful to keep this simple and allow all "reusable" panes are available as blocks. However, there needs to be a global setting to enable this, because I wouldn't want a site to suddenly have hundreds or thousands of records flooding the block system.
Comment #9
Pere OrgaThat sounds good to me
Comment #10
Chris Burge CreditAttribution: Chris Burge commentedAttached is the patch from #6 modified as follows and rolled against latest dev:
(It's possible an entity could be added to a region as a block and later have its reusable status changed to FALSE.)
Comment #11
MiroslavBanov CreditAttribution: MiroslavBanov commentedThe dependency to entity is because these functions are used:
But they can easily be replaced with these instead:
Comment #12
Chris Burge CreditAttribution: Chris Burge commentedRevised patch attached.
When 'fieldable_panels_panes_expose_as_blocks' variable is set to FALSE, the patch from #10 was causing the following error:
This patch corrects this issue by moving '$blocks = array();' and 'return $blocks;' outside the 'if' logic in fieldable_panels_panes_block_info().
Comment #13
Dave ReidFYI entity_label() is in core and *should* be used
Comment #14
Chris Burge CreditAttribution: Chris Burge commented@Dave Reid - I'll update the patch to use entity_label() in the context of this issue. Do you want to create a separate issue to replace the fieldable_panels_panes_entity_label_callback() function elsewhere?
Comment #15
Chris Burge CreditAttribution: Chris Burge commentedRevised patch attached.
Per @Dave Reid, replaces fieldable_panels_panes_entity_label_callback() with entity_label().
Comment #16
MiroslavBanov CreditAttribution: MiroslavBanov commented@Chris Burge - I didn't realize "entity_label()" is in core - it's sometimes quite confusing what part of entity functionality is in core and what isn't. Actually I don't see "fieldable_panels_panes_entity_label_callback" being called directly anywhere, so no need for another issue.
Comment #17
Chris Burge CreditAttribution: Chris Burge commentedUpdated patch to prepend block title with "FPP(%bundle): ". Without this change, a user would have no idea which blocks are provided by FPP. On sites with more than a handful of reusable FPPs, this would get messy very quickly. Views prepends its block titles with "Views: ".
Has anyone tested this patch or the patch from #15 yet?
Comment #18
MiroslavBanov CreditAttribution: MiroslavBanov commentedTested it, and it is working.
One whitespace error found
As for the possibility of lots of unwanted blocks making blocks administration cumbersome, the "Make field entity panes available as blocks" configuration could be made per-bundle, as suggested here: https://www.drupal.org/node/1860118#comment-6818858
Thoughts?
Edit:
This sounds a bit strange to me. Shouldn't it be "Make fieldable panels panes available as blocks" ?
Comment #19
Chris Burge CreditAttribution: Chris Burge commentedWhitespace noted.
I agree with changing the language from "Make field entity panes available as blocks" to "Make fieldable panels panes available as blocks".
I can see that exposing FPPs as block on a per-bundle basis would be very useful. For example, a site builder may designate a single FPP bundle to be exposed as blocks. I'll work to add that functionality this coming week.
Comment #20
Chris Burge CreditAttribution: Chris Burge commentedHere is an updated patch:
Comment #21
MiroslavBanov CreditAttribution: MiroslavBanov commentedLooks great. There are some code-style errors and nitpicks. Coder can easily detect all of them.
But existing code does not exactly comply with all Drupal standards either, so this can be committed without modification.
Comment #22
DamienMcKennaThis is a reasonable approach, IMHO.
A small nitpick - I don't see the benefit of the "master" setting - just go with the per bundle setting. Also, as MiroslavBanov pointed out, there are a few coding standards violations.
Comment #23
Chris Burge CreditAttribution: Chris Burge commented@DamienMcKenna, you state that you "don't see the benefit of the 'master' setting". Do you see a benefit of an "all" setting, which would expose all FPP bundles and override any per-bundle settings?
Comment #24
DamienMcKenna@Chris Burge: eh. I'm second-guessing myself. Stick with the global option, but just clean up the patch and it should be GTG.
BTW did you double-check that the blocks don't show twice on the Add Content popup?
Comment #25
Chris Burge CreditAttribution: Chris Burge commentedAttached patch is cleaned up with Coder.
Regarding blocks appearing twice, I ran the following test:
Neither "Test Block" nor "Test FPP" appears more than once in the "Add Content" pop up.
Comment #26
MiroslavBanov CreditAttribution: MiroslavBanov commentedWith coder, I still see the following, after applying the patch.
fieldable_panels_panes.module
...
includes/admin.inc:
This is an old version of coder, I think is not even very strict.
Comment #27
Chris Burge CreditAttribution: Chris Burge commentedComment #28
MiroslavBanov CreditAttribution: MiroslavBanov commentedInterdiff between #20 and #27.
Comment #29
MiroslavBanov CreditAttribution: MiroslavBanov commentedLooks good.
Comment #30
DamienMcKennaGoing to hold off on this until the next release.
Chris: Would you mind adding a test or two to confirm the functionality works correctly? Thanks.
Comment #32
DamienMcKennaI've committed this, with the addition of an update script to clear the menu cache. Oh, and the settings page now also shows the FPP bundle's machine name in addition to its label.
Thanks everyone!
Comment #33
DamienMcKennaWe'll add tests separately, see #2691577: (ongoing) Write more tests.