-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add wagtail-footnotes #8466
Add wagtail-footnotes #8466
Conversation
e053bd9
to
02998d8
Compare
2d454ec
to
9ff917c
Compare
9ff917c
to
52d7dc7
Compare
@@ -0,0 +1 @@ | |||
<a aria-labelledby="footnotes" href="#footnote-{{ index }}" id="footnote-source-{{ index }}"><sup>{{ index }}</sup></a> |
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.
Does it matter that the aria-labelledby
is for the whole footnotes area and not the specific linked footnote?
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.
It's based on https://www.niquette.ca/articles/accessible-footnotes/#descriptive_markers and similar research, which suggests that this will read as "Link 2 Footnotes" rather than just "Link 2". So, my understanding is that aria-labeledby
will read the destination id
as the label for the link.
In my research, it seems that the general title of the footnotes/references/endnotes/etc section is preferable here to a label specific to the note in question.
This change adds the wagtail-footnotes library and enables footnotes for full width text rich text fields and tables. It also adds the footnotes inline panel to a selection of pages in the admin.
52d7dc7
to
1de23b2
Compare
class RichTextBlockWithFootnotes(WagtailFootnotesRichTextBlockWithFootnotes): | ||
def render_footnote_tag(self, index): | ||
template = get_template(settings.WAGTAIL_FOOTNOTES_REFERENCE_TEMPLATE) | ||
return template.render({"index": index}) | ||
|
||
def replace_footnote_tags(self, value, html, context=None): | ||
# This is a wholesale copy of the replace_footnote_tags() method in | ||
# wagtail-footnotes's RichTextBlockWithFootnotes. It modifies the | ||
# embedded replace_tag() function to call our own | ||
# render_footnote_tag() method. This is a change that should be | ||
# contributed back upstream to allow straight-forward modification of | ||
# footnote link rendering. | ||
# | ||
# There is an alternative implementation proposed in a PR in 2022: | ||
# https://github.com/torchbox/wagtail-footnotes/pull/27 | ||
# But I think I prefer providing a new method to a new embedded func. |
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.
As an FYI, I've opened torchbox/wagtail-footnotes#70 with changes that mirror what I've implemented here, so that hopefully, with feedback, better custom rendering support can be added upstream and we can remove this custom subclass of RichTextBlockWithFootnotes
.
This PR adds:
content_with_footnotes
rich text field supporting footnotes toFullWidthText
rich_text_with_footnotes
rich text field support footnotes toTable
AnswerPage
BlogPage
BrowseFilterablePage
DocumentDetailPage
EnforcementActionPage
EventPage
LearnPage
StoryPage
SublandingFilterablePage
SublandingPage
RichTextBlockWithFootnotes
block.On the last one, wagtail-footnotes renders the reference link with hardcoded HTML that wraps the number in brackets. We don't want brackets. I've subclassed
RichTextBlockWithFootnotes
and extended it to support rendering a template instead. I've done so with an eye toward contributing this modification back to wagtail-footnotes once we have a chance.How to test this PR
Screenshots
On the frontend the intention is that this matches our manually-created footnotes as closely as possible for now:
Checklist