Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Footnotes are not replaced when RichTextWithFootnotesBlock is a StructBlock sub-block #22

Open
ababic opened this issue Nov 19, 2021 · 4 comments

Comments

@ababic
Copy link

ababic commented Nov 19, 2021

When you include a RichTextWithFootnotesBlock in a StreamBlock definition, things work great! This is because StreamBlock.to_python returns a StreamValue, which has a render_as_block() method that feeds back into StreamBlock.render_basic() to do the actual rendering... which calls the render() method of each sub-block, ultimately leading to RichTextWithFootnotesBlock.replace_footnote_tags() doing its thing.

When you include a RichTextWithFootnotesBlock as a sub-block of a StructBlock, things work out a little differently. The tendency is to do the following in StructBlock templates:

{% load wagtailcore_tags %}
{{ value.caption|richtext }}

This definitely won't trigger RichTextWithFootnotesBlock.replace_footnote_tags(), because we're basically just taking the value from the block and rendering it directly.

I thought that changing all instances of the above would resolve the problem:

{% load wagtailcore_tags %}
{% include_block value.caption %}

However, that didn't do the job because {% include_block %} only does anything special with the value if it has a render_as_block() method (like StreamValue does). If not, the value itself is used as the output value, which isn't really what we want.

What we want is for RichTextWithFootnotesBlock.render() to always be used in rendering, and in order to do that, RichTextWithFootnotesBlock needs to return something other than an instance of RichText... we need something with a render_as_block() method (like StreamValue does), so that we can always bring things back to RichTextWithFootnotesBlock.render(). Something like this worked in a project I am currently working on:

class RichTextBlockValue:
    """
    Value type returned by CustomRichTextBlockWithFootnotes.to_python().

    When rendered in a StructBlock, FieldBlock values are usually rendered directly, bypassing the
    footnote replacement logic that is usually triggered by RichTextBlockWithFootnotes.render(). 
    If the block instead returns a value with a 'render_as_block()' method, we can always reliably
    hand things back to RichTextBlockWithFootnotes.render() to take care of rendering, where the
    footnote replacement logic will kick-in.
    """

    def __init__(self, block, value):
        self.block = block
        self.source = value
        self.value = RichText(value)

    def render_as_block(self, context=None):
        return self.block.render(self, context=context)

    def __html__(self):
        return self.block.render(self)

    def __str__(self):
        return self.__html__()


class CustomRichTextWithFootnotesBlock(RichTextWithFootnotesBlock):
    def to_python(self, value):
        if isinstance(value, RichTextBlockValue):
            return value
        return RichTextBlockValue(self, value)

    def render(self, value, context=None):
        if isinstance(value, RichTextBlockValue):
            value = value.value
        return super().render(value, context=context)
   

With these changes, as long as I use {% include_block %} in all StructBlock templates, the footnotes are replaced successfully, wherever they are used:

{% load wagtailcore_tags %}
{% include_block value.caption %}
@ababic ababic changed the title Footnotes are not replaced when RichTextWithFootnotesBlock is a StructBlock field Footnotes are not replaced when RichTextWithFootnotesBlock is a StructBlock sub-block Nov 19, 2021
@ababic
Copy link
Author

ababic commented Nov 30, 2021

I just realised that there is a simpler way to get RichTextWithFootnotesBlock.replace_footnote_tags() to run for StructBlock sub-blocks, which does not require customisation of the return value:

{% load wagtailcore_tags %}
{% include_block value.bound_blocks.caption %}

With this, {% include_block %} has access to the block AND value, and can use the former to render the latter.

@CamLamb
Copy link

CamLamb commented Nov 30, 2021

Interesting, I do think that it'd be great if we could standardise the RichTextWithFootnotesBlock so that you don't need to be aware of these different uses.

Personally, after an initial skim of your suggestion, I quite like the idea of creating aRichTextBlockValue as it sounds like it makes the Block function more like the existing Blocks.

@ababic
Copy link
Author

ababic commented Dec 1, 2021

Personally, after an initial skim of your suggestion, I quite like the idea of creating a RichTextBlockValue as it sounds like it makes the Block function more like the existing Blocks.

One downside of this approach is that {{ value.caption|richtext }} raises an exception (because richtext expects a str or RichText value only), whereas the {% include_block value.bound_blocks.caption %} solution just works (the 'bound blocks' trick is mentioned in the wagtail docs here, but it's quite easy to miss)

@jsma
Copy link
Contributor

jsma commented Feb 24, 2022

I was bitten by this the other day.

the 'bound blocks' trick is mentioned in the wagtail docs here, but it's quite easy to miss

Agreed. I'm not sure if I had read that specific section of the docs before but if I did, I completely forgot about it. I eventually stumbled upon them and was able to solve my issue by using bound_blocks. At any rate, is the only viable solution simply to document the StructBlock gotcha here as well? If so, I can work up a PR to update the docs. Let me know. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants