- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Setting correct status. Will test this in a bit!
- Status changed to RTBC
over 1 year ago 10:15am 10 May 2023 - 🇧🇪Belgium BramDriesen Belgium 🇧🇪
The event subscriber works as intended. Tagging for a documentation update and/or release note.
Small nit, we might want to leave this todo in the code as it's still valid.
+++ b/src/EntityAnalyser.php @@ -137,11 +150,20 @@ class EntityAnalyser { - // @todo Make the view mode configurable in Yoast SEO settings.
- last update
over 1 year ago 3 pass - 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Adding a patch combining this issue and 🐛 Analyze frontend theme output + issue with missing elements when analyzing Fixed . This patch should not be used to implement this issue.
- Status changed to Postponed: needs info
over 1 year ago 10:20pm 3 June 2023 - 🇳🇱Netherlands kingdutch
Thanks for the proposed fix! I'm hesitant to implement it in the current format. We're using the event system here to replace the implementation of the
renderEntity
method. Since if any of the event listeners produces HTML, we don't run the default implementation of the function anymore.For the use-case I can come up with I would expect a single implementation of the event listener. In that case why would using Drupal's service system to replace the
yoast_seo.entity_analyser
not be sufficient? The method is purposefully self contained so that it can be easily overwritten by a child class (which can optionally fallback to the parent method).Could you describe situations in which the above service-based solution doesn't work and adding the event system provides benefits?
[Hiding Bram's patch since it's there for convenience and not to implement the issue :)]