Problem/Motivation
Reopening this issue as 3334147 was closed but the problem has not been resolved as missing patch #4.
Proposed resolution
Attached patch works as expected and should be released.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | entity_extra_field_3469612-5.patch | 2.19 KB | droath |
Issue fork entity_extra_field-3469612
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
anybody@coaston how do you come to this conclusion?
Comparing the commit from the linked issue
https://git.drupalcode.org/project/entity_extra_field/-/commit/94a2ab3dc...
to the latest branch:
https://git.drupalcode.org/project/entity_extra_field/-/blob/2.1.x/src/P...
it's present!
Comment #3
starlight-sparkleCoaston means to say that the original commit is incomplete, not that the commit is missing.
The MR created by jnettik implemented the fix in a different way from the patch file submitted by artyom. Namely:
Element::isEmpty()This is significant in the special case of views because as explained:
Comment #4
joelsteidl commentedI can confirm that this patch solved the issue of a View block with empty output. I will take a look at the approach and see if it can be improved.
Comment #5
droath commentedUpon further reflection on this issue, there are two potential solutions for using Entity Extra Fields to render Views. A developer can implement them using either the Views or the Block plugin within the Entity Extra Field module. I currently do not recommend supporting views being rendered in the block plugin when we have a Views plugin available. I've added some additional code to the Views Plugin within the Entity Extra Field module to address this issue. Please let me know if this addresses the issue you're personally experiencing.
Comment #6
droath commentedComment #7
coaston commentedThank you, your patch works as expected with the latest 2.1.0-rc4 version. No issue found. I think it is ready to be merged.
Comment #8
anybody@coaston thanks for the RTBC. Could you please create a MR from the patch? Thanks!
Comment #9
divyansh.gupta commentedWorking on it!!
Comment #11
divyansh.gupta commentedConverted the patch to MR,
please review!!
Comment #12
anybodyThanks for the MR. Back to NR, would be nice if someone here could test it (or even better add a test)!
Code-wise this LGTM, still there might be an impact on performance, as each view is now executed twice? Do we have a way to improve that or are there better ways?
Otherwise I'd be fine to merge this after review. Thank you! :)
Comment #13
anybodyComment #14
grevil commentedYes, this is indeed a problem, but I am unsure how to fix it. As @starlight-sparkle in #3 correctly says, that this is probably the only way to check whether the output is really empty or not.
I thought about caching, but I don't think that is a good option in this context...
Comment #15
grevil commentedI guess this is the best approach. Code LGTM!
Comment #17
grevil commented