-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow multiple references to the same footnote #40
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can do away with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are two different things. I didn't have any better idea of how to make the Footnote objects aware of the one or more references contained in the rich text blocks. This is why I made the goofy template tag so I could at least do dict lookups using the UUID in the template to fetch the references. I'm working on getting this rebased on main now that the custom template PR was merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, that makes sense. I think some additional comments would be helpful then, and perhaps some type hinting, then there will be no ambiguity. Thank you |
||
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'<a href="#footnote-{index}" id="footnote-source-{index}"><sup>[{index}]</sup></a>' | ||
# 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])}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: keep indexes consistent. for regular users 1-1, 1-2 etc make more sense than 1-0. 1-1 |
||
page.footnotes_references[footnote_uuid].append(link_id) | ||
return f'<a href="#footnote-{index}" id="{link_id}"><sup>[{index}]</sup></a>' | ||
|
||
# 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): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure we need this. if we drop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
[{uuid[:6]}]
bit isn't strictly required sinceFIND_FOOTNOTE_TAG
doesn't care what's between<footnote></footnote>
but I opted to make it an accurate representation of what is actually stored in the database.