From 5a9441238dcc503f5bffdafbad069144fd16f993 Mon Sep 17 00:00:00 2001 From: John-Scott Atlakson <24574+jsma@users.noreply.github.com> Date: Fri, 14 Mar 2025 15:27:27 -0700 Subject: [PATCH] Allow multiple references to the same footnote Previously every link in the rich text for the same footnote had the same `id` attribute, which breaks the "Back to content" link (or just always returns the user to the first reference). This now generates a unique `id` for each footnote reference and updates the `footnotes.html` template to generate unique links back to each reference in the content. --- tests/templates/test/endnote_reference.html | 2 +- tests/test/test_blocks.py | 68 ++++++++++++++---- tests/test/test_functional.py | 8 +-- tests/test/test_translation.py | 8 +-- wagtail_footnotes/blocks.py | 71 +++++++++++++------ .../includes/footnote_reference.html | 2 +- .../wagtail_footnotes/includes/footnotes.html | 23 +++++- 7 files changed, 132 insertions(+), 50 deletions(-) diff --git a/tests/templates/test/endnote_reference.html b/tests/templates/test/endnote_reference.html index b4b31b9..801e709 100644 --- a/tests/templates/test/endnote_reference.html +++ b/tests/templates/test/endnote_reference.html @@ -1 +1 @@ -{{ index }} +{{ index }} diff --git a/tests/test/test_blocks.py b/tests/test/test_blocks.py index 85e4492..bcb322a 100644 --- a/tests/test/test_blocks.py +++ b/tests/test/test_blocks.py @@ -38,19 +38,47 @@ def setUp(self): [ { "type": "paragraph", - "value": f'

This is a paragraph with a footnote. 1

', + "value": ( + f'

This is a paragraph with a footnote. [{uuid[:6]}]

' + ), }, ] ), ) home_page.add_child(instance=self.test_page_with_footnote) self.test_page_with_footnote.save_revision().publish() - self.footnote = Footnote.objects.create( + Footnote.objects.create( page=self.test_page_with_footnote, uuid=uuid, text="This is a footnote", ) + self.test_page_with_multiple_references_to_the_same_footnote = TestPageStreamField( + title="Test Page With Multiple References to the Same Footnote", + slug="test-page-with-multiple-references-to-the-same-footnote", + body=json.dumps( + [ + { + "type": "paragraph", + "value": ( + f'

This is a paragraph with a footnote. [{uuid[:6]}]

' + f"

This is another paragraph with a reference to the same footnote. " + f'[{uuid[:6]}]

' + ), + }, + ] + ), + ) + home_page.add_child( + instance=self.test_page_with_multiple_references_to_the_same_footnote + ) + self.test_page_with_multiple_references_to_the_same_footnote.save_revision().publish() + Footnote.objects.create( + page=self.test_page_with_multiple_references_to_the_same_footnote, + uuid=uuid, + text="This is a footnote", + ) + def test_block_with_no_features(self): block = RichTextBlockWithFootnotes() self.assertIsInstance(block, blocks.RichTextBlock) @@ -98,27 +126,39 @@ def test_block_replace_footnote_tags(self): html = block.replace_footnote_tags(None, "foo") self.assertEqual(html, "foo") - def test_block_replace_footnote_render_basic(self): + def test_block_replace_footnote_render(self): rtb = self.test_page_with_footnote.body.stream_block.child_blocks["paragraph"] value = rtb.get_prep_value(self.test_page_with_footnote.body[0].value) context = self.test_page_with_footnote.get_context(self.client.get("/")) - out = rtb.render_basic(value, context=context) - result = '

This is a paragraph with a footnote. [1]

' + out = rtb.render(value, context=context) + result = ( + '

This is a paragraph with a footnote. [1]' + "

" + ) self.assertHTMLEqual(out, result) - def test_block_replace_footnote_render(self): - rtb = self.test_page_with_footnote.body.stream_block.child_blocks["paragraph"] - value = rtb.get_prep_value(self.test_page_with_footnote.body[0].value) - context = self.test_page_with_footnote.get_context(self.client.get("/")) + def test_block_replace_footnote_with_multiple_references_render(self): + body = self.test_page_with_multiple_references_to_the_same_footnote.body + rtb = body.stream_block.child_blocks["paragraph"] + value = rtb.get_prep_value(body[0].value) + context = ( + self.test_page_with_multiple_references_to_the_same_footnote.get_context( + self.client.get("/") + ) + ) out = rtb.render(value, context=context) - result = '

This is a paragraph with a footnote. [1]

' + result = ( + '

This is a paragraph with a footnote. [1]' + '

This is another paragraph with a reference to the same footnote. [1]

' + ) self.assertHTMLEqual(out, result) def test_render_footnote_tag(self): block = RichTextBlockWithFootnotes() - html = block.render_footnote_tag(2) + html = block.render_footnote_tag(index=2, reference_index=1) self.assertHTMLEqual( - html, '[2]' + html, '[2]' ) @override_settings( @@ -126,7 +166,7 @@ def test_render_footnote_tag(self): ) def test_render_footnote_tag_new_template(self): block = RichTextBlockWithFootnotes() - html = block.render_footnote_tag(2) + html = block.render_footnote_tag(index=2, reference_index=1) self.assertHTMLEqual( - html, '2' + html, '2' ) diff --git a/tests/test/test_functional.py b/tests/test/test_functional.py index 2f3ee1e..4a32b9b 100644 --- a/tests/test/test_functional.py +++ b/tests/test/test_functional.py @@ -78,21 +78,21 @@ def test_with_footnote(self): # Test that required html tags are present with correct # attrs that enable the footnotes to respond to clicks - source_anchor = soup.find("a", {"id": "footnote-source-1"}) + source_anchor = soup.find("a", {"id": "footnote-source-1-1"}) self.assertTrue(source_anchor) source_anchor_string = str(source_anchor) self.assertIn("[1]", source_anchor_string) self.assertIn('href="#footnote-1"', source_anchor_string) - self.assertIn('id="footnote-source-1"', source_anchor_string) + self.assertIn('id="footnote-source-1-1"', source_anchor_string) footnotes = soup.find("div", {"class": "footnotes"}) self.assertTrue(footnotes) footnotes_string = str(footnotes) self.assertIn('id="footnote-1"', footnotes_string) - self.assertIn('href="#footnote-source-1"', footnotes_string) - self.assertIn("[1] This is a footnote", footnotes_string) + self.assertIn('href="#footnote-source-1-1"', footnotes_string) + self.assertIn("This is a footnote", footnotes_string) def test_edit_page_with_footnote(self): self.client.force_login(self.admin_user) diff --git a/tests/test/test_translation.py b/tests/test/test_translation.py index c08bef6..6907d14 100644 --- a/tests/test/test_translation.py +++ b/tests/test/test_translation.py @@ -138,18 +138,18 @@ def test_translated_page_shows_translated_footnote(self): # Test that required html tags are present with correct # attrs that enable the footnotes to respond to clicks - source_anchor = soup.find("a", {"id": "footnote-source-1"}) + source_anchor = soup.find("a", {"id": "footnote-source-1-1"}) self.assertTrue(source_anchor) source_anchor_string = str(source_anchor) self.assertIn("[1]", source_anchor_string) self.assertIn('href="#footnote-1"', source_anchor_string) - self.assertIn('id="footnote-source-1"', source_anchor_string) + self.assertIn('id="footnote-source-1-1"', source_anchor_string) footnotes = soup.find("div", {"class": "footnotes"}) self.assertTrue(footnotes) footnotes_string = str(footnotes) self.assertIn('id="footnote-1"', footnotes_string) - self.assertIn('href="#footnote-source-1"', footnotes_string) - self.assertIn("[1] This is a French translated footnote", footnotes_string) + self.assertIn('href="#footnote-source-1-1"', footnotes_string) + self.assertIn("This is a French translated footnote", footnotes_string) diff --git a/wagtail_footnotes/blocks.py b/wagtail_footnotes/blocks.py index 315fbc6..941f0ed 100644 --- a/wagtail_footnotes/blocks.py +++ b/wagtail_footnotes/blocks.py @@ -1,12 +1,13 @@ import re from django.conf import settings -from django.core.exceptions import ValidationError from django.template.loader import get_template from django.utils.safestring import mark_safe from wagtail.blocks import RichTextBlock from wagtail.models import Page +from wagtail_footnotes.models import Footnote + FIND_FOOTNOTE_TAG = re.compile(r'.*?') @@ -15,11 +16,13 @@ class RichTextBlockWithFootnotes(RichTextBlock): """ Rich Text block that renders footnotes in the format 'short-id' as anchor elements. It also - adds the Footnote object to the 'page' object for later use. It uses + adds the Footnote object(s) to the 'page' object for later use. It uses 'page' because variables added to 'context' do not persist into the final template context. """ + all_footnotes: dict[str, Footnote] + def __init__(self, **kwargs): super().__init__(**kwargs) if not self.features: @@ -27,14 +30,14 @@ def __init__(self, **kwargs): if "footnotes" not in self.features: self.features.append("footnotes") - def render_footnote_tag(self, index): + def render_footnote_tag(self, index: int, reference_index: int): template_name = getattr( settings, "WAGTAIL_FOOTNOTES_REFERENCE_TEMPLATE", "wagtail_footnotes/includes/footnote_reference.html", ) template = get_template(template_name) - return template.render({"index": index}) + return template.render({"index": index, "reference_index": reference_index}) def replace_footnote_tags(self, value, html, context=None): if context is None: @@ -42,42 +45,64 @@ def replace_footnote_tags(self, value, html, context=None): else: new_context = self.get_context(value, parent_context=dict(context)) - if not isinstance(new_context.get("page"), Page): + page = new_context.get("page") + if page is None or not isinstance(page, Page): return html - page = new_context["page"] - if not hasattr(page, "footnotes_list"): - page.footnotes_list = [] - self.footnotes = { + # Map Footnote UUIDs to Footnote instances to simplify lookups once a reference has been found in the text. + # NOTE: Footnotes may exist in the database for a given page but this does not necessarily mean that the + # footnote was referenced in the text. + self.all_footnotes = { str(footnote.uuid): footnote for footnote in page.footnotes.all() } + # Patch the page to track the footnotes that are actually referenced in the text, so that they can be rendered + # in footnotes.html + if not hasattr(page, "footnotes_list"): + page.footnotes_list = [] + def replace_tag(match): + footnote_uuid = match.group(1) try: - index = self.process_footnote(match.group(1), page) - except (KeyError, ValidationError): + footnote = self.attach_footnote_to_page(footnote_uuid, page) + except KeyError: return "" else: - return self.render_footnote_tag(index) + # Add 1 to the footnote index as footnotes are rendered in footnotes.html using `{{ forloop.counter }}` + # which is 1-based. + footnote_index = page.footnotes_list.index(footnote) + 1 + reference_index = footnote.references[-1] + # Supplying both indexes allows for unique id values to be generated in the HTML. E.g., the first + # reference to the first footnote will have `id="footnote-source-1-1"`, and the second reference to the + # first footnote will have `id="footnote-source-1-2"`, etc. + return self.render_footnote_tag(footnote_index, reference_index) # note: we return safe html return mark_safe(FIND_FOOTNOTE_TAG.sub(replace_tag, html)) # noqa: S308 def render(self, value, context=None): - if not self.get_template(value=value, context=context): - return self.render_basic(value, context=context) - html = super().render(value, context=context) return self.replace_footnote_tags(value, html, context=context) - def render_basic(self, value, context=None): - html = super().render_basic(value, context) + def attach_footnote_to_page(self, footnote_uuid: str, page: Page) -> Footnote: + """Finds the Footnote object matching `footnote_uuid`, then modifies it to track how many times it has been + referenced, and attaches it to the `page` so the footnote can be rendered in the page template. + """ + # Fetch the unmodified Footnote + footnote = self.all_footnotes[footnote_uuid] - return self.replace_footnote_tags(value, html, context=context) - - def process_footnote(self, footnote_id, page): - footnote = self.footnotes[footnote_id] + # If this is the first time the Footnote has been referenced, modify it to track references before appending it + # to the page if footnote not in page.footnotes_list: + footnote.references = [1] page.footnotes_list.append(footnote) - # Add 1 to the index as footnotes are indexed starting at 1 not 0. - return page.footnotes_list.index(footnote) + 1 + else: + # If this Footnote has been processed by a previous reference, fetch the modified Footnote from the page and + # update its reference tracking + footnote_index = page.footnotes_list.index(footnote) + footnote = page.footnotes_list[footnote_index] + # Update the references e.g., [1, 2] + footnote.references.append(footnote.references[-1] + 1) + # Update the page with the updated footnote + page.footnotes_list[footnote_index] = footnote + return footnote diff --git a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnote_reference.html b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnote_reference.html index 86810ae..49bf4be 100644 --- a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnote_reference.html +++ b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnote_reference.html @@ -1 +1 @@ -[{{ index }}] +[{{ index }}] diff --git a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html index 4878987..79b4af6 100644 --- a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html +++ b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html @@ -7,10 +7,27 @@

    {% for footnote in page.footnotes_list %} -
  1. - [{{ forloop.counter }}] {{ footnote.text|richtext }} - ↩ + {% with footnote_index=forloop.counter %} +
  2. + {% if footnote.references|length == 1 %} + {# If there is only a single reference, link the return icon back to it #} + + ↩ + + {% else %} + ↩ + {% for reference_index in footnote.references %} + {# If there are multiple references, generate unique links per reference #} + + {# Display a 1-indexed counter for each of the references to this footnote #} + {{ forloop.counter }} + + + {% endfor %} + {% endif %} + {{ footnote.text|richtext }}
    3. + {% endwith %} {% endfor %}