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

CKEditor footnotes #4419

Merged
merged 87 commits into from Jan 3, 2022
Merged

CKEditor footnotes #4419

merged 87 commits into from Jan 3, 2022

Conversation

jcmustin
Copy link
Collaborator

@jcmustin jcmustin commented Dec 27, 2021

This PR adds footnote support to CKEditor. (Note: this PR predated Forum Magnum, and has already gone through review within CEA here.)

Footnotes can be added in two ways:

  • via the popup toolbar, when content is selected
  • using markdown syntax ([^<number>]), which gets auto-converted to the same format as the above methods
    Footnotes are indexed 1 to n. Deleting a footnote renumbers subsequent footnotes and their references. Reordering footnote references in the post reorders footnotes in the footer (s.t. the first footnote reference always has index 1).

See the README for a more thorough description of functionality and specifics about the code.

jcmustin and others added 30 commits October 20, 2021 13:56
Co-authored-by: jcmustin <jonathan.mustin@centreforeffectivealtruism.org>
Co-authored-by: JP Addison <johnpaddison@gmail.com>
@jcmustin jcmustin requested a review from a team as a code owner December 27, 2021 17:58
@jcmustin jcmustin requested review from jpaddison3 and jimrandomh and removed request for a team December 27, 2021 17:58
@Raemon
Copy link
Collaborator

Raemon commented Dec 28, 2021

I tested this out. When I select a word, and click "add footnote", it seems to insert the footnote indicator at the beginning of the word rather than the end, which is what I'd expect and want.

@jcmustin
Copy link
Collaborator Author

I tested this out. When I select a word, and click "add footnote", it seems to insert the footnote indicator at the beginning of the word rather than the end, which is what I'd expect and want.

Thank you! Fixed in 940827e.

@darkruby501
Copy link
Collaborator

Very cool! This is one of the most requested features, so I'm very glad to see this being shipped. Thanks.

Thoughts:

  • My understanding is that the side toolbar is for block/paragraph level things, which this isn't it. So both for conceptual reasons and for reasons of redundancy, I'd remove it from the side/block menu and only have it for the pop-up select-text menu.
  • It would be super cool and I think worth delaying this to introduce hover-over preview for footnotes.

@jcmustin
Copy link
Collaborator Author

jcmustin commented Dec 30, 2021

  • My understanding is that the side toolbar is for block/paragraph level things, which this isn't it. So both for conceptual reasons and for reasons of redundancy, I'd remove it from the side/block menu and only have it for the pop-up select-text menu.

That makes sense! Removed from the block toolbar in 45a1fad.

It would be super cool and I think worth delaying this to introduce hover-over preview for footnotes.

Added in 45a1fad + 664023a 🙂

Copy link
Collaborator

@jpaddison3 jpaddison3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (already reviewed in old PR)

I would make sure to double check that the transpiled code is up-to-date before merging.

@jcmustin jcmustin merged commit 171899b into master Jan 3, 2022
3 checks passed
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

Successfully merging this pull request may close these issues.

None yet

4 participants