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

Footnotes inside panels don't work #972

Closed
damithc opened this issue Jan 10, 2020 · 10 comments · Fixed by #1403
Closed

Footnotes inside panels don't work #972

damithc opened this issue Jan 10, 2020 · 10 comments · Fixed by #1403

Comments

@damithc
Copy link
Contributor

damithc commented Jan 10, 2020

Example (see the footnote inside the tutorials panel) https://nus-cs2113-ay1920s2.github.io/website/admin/weeklySchedule.html

image

@openorclose
Copy link
Contributor

This occurs as the panel is a dynamic panel, so the footnotes are not generated in the "main" html.

We could alternatively put footnotes at the bottom referencing stuff in unloaded panels. Of course, the downside would be that the reader might see a sudden jump in footnote numbering due to unloaded panels.

Implementation wise, we would have to loop through all nested panels to exhaustively find all footnotes.

@damithc
Copy link
Contributor Author

damithc commented Jan 12, 2020

If we assume that panels are typically used to embed content taken from elsewhere, it makes more sense if the footnote is contained inside the panel rather than at the bottom of the host page. Is that feasible?

@openorclose
Copy link
Contributor

Yeah, that would be feasible

@damithc
Copy link
Contributor Author

damithc commented Jan 12, 2020

Yeah, that would be feasible

Great. Let's give that a try then.

@openorclose
Copy link
Contributor

openorclose commented Feb 2, 2020

@damithc I remember the reason we removed the "linkbacks" from the footnotes was because sometimes the panel containing the reference wouldn't be open, so it would be confusing if the linkback took you nowhere.

Since now each panel has its own footnote, would it be good to add the linkbacks back?

image

referring to the arrow beside "Math".

@damithc
Copy link
Contributor Author

damithc commented Feb 2, 2020

Since now each panel has its own footnote, would it be good to add the linkbacks back?

Sure 👍

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Aug 8, 2020

Since now each panel has its own footnote, would it be good to add the linkbacks back?

Sure 👍

Since we show the footnotes in popovers, the backreference would also appear in the popover though.
I think the popover implementation is sufficient to warrant not adding a backreference, since you can directly see the footnote. The bottom footnote area then just serves as a "footnote consolidation area".

The back button in browsers should also be able to take you back to where you were before clicking the footnote and is also more accurate if you have multiple references to the same footnote (otherwise it always takes you back to the topmost footnote reference)

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Aug 8, 2020

Since footnotes for includes from panels are now "sealed" as well, should we also clarify how they should behave in the case of static <include>s?

  • should footnote definitions from <include>s be allowed to leak into the content
  • should the content's footnote definitions be allowed to leak into the <include>
  • footnotes from static includes should display together with the main content's footnotes, unlike panels

@damithc
Copy link
Contributor Author

damithc commented Aug 8, 2020

@openorclose any thoughts?

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Dec 1, 2020

I'm going to encapsulate (1) and (2) for the v3 'rewrite' for now, it may lead to easier-to-predict behaviour (would also fix this issue)

Footnote definitions can still be reused via nunjucks' {% include %} which is preprocessed.
It'll be a long way to a proper v3 release so we can change things as needed as well 🙂

(3) should definitely be a thing though

@ang-zeyu ang-zeyu mentioned this issue Dec 11, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants