From 6990138ec7ca22491c7ca9403476979554e27452 Mon Sep 17 00:00:00 2001 From: John-Scott Atlakson <24574+jsma@users.noreply.github.com> Date: Sun, 30 Jul 2023 12:58:09 +0900 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/test/test_blocks.py | 59 +++++++++++++++++-- tests/test/test_functional.py | 8 +-- wagtail_footnotes/blocks.py | 19 +++++- .../wagtail_footnotes/includes/footnotes.html | 20 ++++++- wagtail_footnotes/templatetags/__init__.py | 0 .../templatetags/wagtailfootnotes_tags.py | 15 +++++ 6 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 wagtail_footnotes/templatetags/__init__.py create mode 100644 wagtail_footnotes/templatetags/wagtailfootnotes_tags.py diff --git a/tests/test/test_blocks.py b/tests/test/test_blocks.py index ade57f0..40cf26c 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) @@ -103,7 +131,10 @@ def test_block_replace_footnote_render_basic(self): 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]

' + result = ( + '

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

" + ) self.assertEqual(out, result) def test_block_replace_footnote_render(self): @@ -111,5 +142,25 @@ def test_block_replace_footnote_render(self): 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(value, context=context) - result = '

This is a paragraph with a footnote. [1]

' + result = ( + '

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

" + ) + self.assertEqual(out, result) + + 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]' + '

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

' + ) self.assertEqual(out, result) diff --git a/tests/test/test_functional.py b/tests/test/test_functional.py index 9c25588..81c0623 100644 --- a/tests/test/test_functional.py +++ b/tests/test/test_functional.py @@ -76,21 +76,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-0"}) 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-0"', 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-0"', 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/wagtail_footnotes/blocks.py b/wagtail_footnotes/blocks.py index 71b17da..e81b8b3 100644 --- a/wagtail_footnotes/blocks.py +++ b/wagtail_footnotes/blocks.py @@ -1,5 +1,7 @@ import re +from collections import defaultdict + from django.core.exceptions import ValidationError from django.utils.safestring import mark_safe from wagtail.blocks import RichTextBlock @@ -24,6 +26,7 @@ def __init__(self, **kwargs): self.features = [] if "footnotes" not in self.features: self.features.append("footnotes") + self.footnotes = {} def replace_footnote_tags(self, value, html, context=None): if context is None: @@ -37,17 +40,28 @@ def replace_footnote_tags(self, value, html, context=None): page = new_context["page"] if not hasattr(page, "footnotes_list"): page.footnotes_list = [] + if not hasattr(page, "footnotes_references"): + page.footnotes_references = defaultdict(list) self.footnotes = { str(footnote.uuid): footnote for footnote in page.footnotes.all() } def replace_tag(match): + footnote_uuid = match.group(1) try: - index = self.process_footnote(match.group(1), page) + index = self.process_footnote(footnote_uuid, page) except (KeyError, ValidationError): return "" else: - return f'[{index}]' + # Generate a unique html id for each link in the content to this footnote since the same footnote may be + # referenced multiple times in the page content. For the first reference to the first footnote, it will + # be "footnote-source-1-0" (the index for the footnote is 1-based but the index for the links are + # 0-based) and if it's the second link to the first footnote, it will be "footnote-source-1-1", etc. + # This ensures the ids are unique throughout the page and allows for the template to generate links from + # the footnote back up to the distinct references in the content. + link_id = f"footnote-source-{index}-{len(page.footnotes_references[footnote_uuid])}" + page.footnotes_references[footnote_uuid].append(link_id) + return f'[{index}]' # note: we return safe html return mark_safe(FIND_FOOTNOTE_TAG.sub(replace_tag, html)) # noqa: S308 @@ -61,7 +75,6 @@ def render(self, value, context=None): def render_basic(self, value, context=None): html = super().render_basic(value, context) - return self.replace_footnote_tags(value, html, context=context) def process_footnote(self, footnote_id, page): diff --git a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html index 275dd24..8ce9bcd 100644 --- a/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html +++ b/wagtail_footnotes/templates/wagtail_footnotes/includes/footnotes.html @@ -1,4 +1,5 @@ {% load wagtailcore_tags %} +{% load wagtailfootnotes_tags %} {% if page.footnotes_list %}
@@ -8,8 +9,23 @@

    {% for footnote in page.footnotes_list %}
  1. - [{{ forloop.counter }}] {{ footnote.text|richtext }} - ↩ + {% with reference_ids=page|get_reference_ids:footnote.uuid num=reference_ids|length %} + {% if num == 1 %}{# If there is only a single reference, simply link the return icon back to it #} + + ↩ + + {% else %} + ↩ + {% for reference_id in reference_ids %} + + {# Display a 1-indexed counter for each of the references to this footnote #} + {{ forloop.counter }} + + + {% endfor %} + {% endif %} + {% endwith %} + {{ footnote.text|richtext }}
    2. {% endfor %}
diff --git a/wagtail_footnotes/templatetags/__init__.py b/wagtail_footnotes/templatetags/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/wagtail_footnotes/templatetags/wagtailfootnotes_tags.py b/wagtail_footnotes/templatetags/wagtailfootnotes_tags.py new file mode 100644 index 0000000..4e34638 --- /dev/null +++ b/wagtail_footnotes/templatetags/wagtailfootnotes_tags.py @@ -0,0 +1,15 @@ +from django import template + + +register = template.Library() + + +@register.filter +def get_reference_ids(value, footnote_uuid): + """This takes the current `footnote_uuid` and returns the list of references in the page content to that footnote. + This template tag is only necessary because it is not possible to do dictionary lookups using variables as keys in + Django templates. + """ + if hasattr(value, "footnotes_references"): + return value.footnotes_references.get(footnote_uuid, []) + return []