Skip to content

feat(Message): Add support for footnotes - #648

Merged
rebeccaalpert merged 8 commits into
patternfly:mainfrom
rebeccaalpert:footnotes
Sep 5, 2025
Merged

rebeccaalpert merged 8 commits into
patternfly:mainfrom
rebeccaalpert:footnotes

Conversation

@rebeccaalpert

@rebeccaalpert rebeccaalpert commented Aug 8, 2025

Copy link
Copy Markdown
Member

Styled footnotes and added demos with some onClick events. Used highlighting rather than scrolling - let me know if you have feelings on this.

Tested click behavior function for messages in main ChatBot demo as well and it worked ok in basic demo.

In smart scroll demo, got it to work with small mods (not shipping):

  • Add const footnoteNavigationActive = React.useRef(false);
  • Change layoutEffect to if (!messageBoxRef.current?.isSmartScrollActive() || scrollQueued.current || footnoteNavigationActive.current) {
  • Add footnote logic to shouldUpdate: if (shouldUpdate && !footnoteNavigationActive.current) {
  • Wrap this bit in a check for the footnotes:
     if (!footnoteNavigationActive.current) {
        messageBoxRef.current?.scrollToBottom({ resumeSmartScroll: true });
      }

Did not perfect scrolling in smart scroll demo - just used default focus without preventScroll. I figure this would be up to products what they want to do/not all may want to use backrefs this way.

Assisted-by: Cursor (used for debugging demos, copying bot work to user message demo, and removing node from all components except table -- these were getting passed to the DOM as objects and cluttering things up)

Demos (just pick "footnote" from select):

@rebeccaalpert rebeccaalpert linked an issue Aug 8, 2025 that may be closed by this pull request
@patternfly-build

patternfly-build commented Aug 8, 2025

Copy link
Copy Markdown

@rebeccaalpert
rebeccaalpert force-pushed the footnotes branch 9 times, most recently from a54f1dd to 071d590 Compare August 12, 2025 15:50
@rebeccaalpert rebeccaalpert changed the title Add basic footnotes feat(Message): Add support for footnotes Aug 12, 2025
@rebeccaalpert
rebeccaalpert marked this pull request as ready for review August 12, 2025 17:32

@kaylachumley kaylachumley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no specific design notes! wasn't sure if the footnote text should start inline with the numbers instead of on the line below the number? i'll leave that up to erin!

@edonehoo edonehoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a couple thoughts on the content in the example!

Do we have any control over the return to original text arrows?
image

If so, I feel like the use of a footnote style there too is a little confusing -- since the second arrow has a "2" footnote, but really just links to the 2nd occurrence of footnote 1. I can offer more specific suggestions if others agree and it is something we can customize, but I couldn't tell

@rebeccaalpert

Copy link
Copy Markdown
Member Author

Do we have any control over the return to original text arrows?

We could hook into the "data-footnote-backref" class that gets applied by the Markdown -> HTML conversion to hide superscripts (sup tag) or (larger effort) write some JavaScript to manually modify the DOM tree.

We get what's there largely for free from https://github.com/remarkjs/remark-gfm, which is based on GitHub's defaults.

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some general comments in testing out the examples:

  • Not sure we should visually hide the "Footnotes" heading. Visually having it present might help make it immediately clear what that message contains, whereas if there's a lengthy message with several footnote links, when you finally get to the message containing the footnotes content the connection might be lost.
  • Are consumers able to customize the content of the footnotes headding and id? We should alloww for that in the case of several footnote messages where maybe the headings should be "Footnotes for Message from 8:05"/"Footnotes for [some synopsis of the convo)"
  • Also not sure about the background color of the message changing when focus is moved when clicking a footnote link - might be a bit unexpected?
  • For the "User" messages, clicing the "1" footnote in the first message works as expected. Clicking the "2", however, focus gets lost. Then when clicking the "1" in the second message works as expected, but clicking the link to "go back" moves focus to the first message - I would expect focus to go to the second message's "1" footnote link.
  • The "go back" links in the footnote content is also a bit iffy - clicking the link with a "2" in it brings me back to the second message "1" footnote link, and the last "go back" link brings me to the first messages "2" footnote link
  • You had mentioned scrolling in the smart scroll demo, not sure if that comment included other examples like the basic "User messages" example. In that the page doesn't scroll so that the focused item is in view - that's something we should resolve even if it's only in example docs.
  • Can we include some verbiage about footnote links with the same content should link to the same thing? I.e. the "1" footnote links should both go to the same footnote content item, and "go back" links similarly similarly should go back to the same footnote link?
  • The labeling of the "go back" links seems a little off? "Back to reference 1" is fine, but then the next one with a "2" visually is "Back to reference 1 - 2", and the last one (which looks the same as the first one) says "Back to reference 2"
  • The focus logic when going from a "go back" link to a footnote link is mostly as I'd expect, but I'm not sure if focus should go to the "go back" link when clicking a footnote link. That may not allow a user to easily digest the footnote content without having to navigate backwards. Might be better to give a -1 tabindex on the li and programmatically focus that when clicking a footnote link

Not exactly sure how much of this we can easily fix, but I think a lot of it is stuff we need to resolve even if it's example-only code before shipping implementations.

@rebeccaalpert

Copy link
Copy Markdown
Member Author

Users should be able to adjust headers/ids using any of these options here: https://github.com/remarkjs/remark-rehype?tab=readme-ov-file#fields

Example on a Message: reactMarkdownProps={{remarkRehypeOptions: {clobberPrefix: 'test'}}}

Screenshot 2025-08-20 at 2 04 39 PM

@rebeccaalpert

Copy link
Copy Markdown
Member Author

Addressed style changes we discussed @kaylachumley @thatblindgeye @edonehoo!

@rebeccaalpert

Copy link
Copy Markdown
Member Author

Just rebasing.

rebeccaalpert and others added 7 commits September 5, 2025 09:26
Styled footnotes and added demos with some onClick events.

Assisted-by: Cursor (used for debugging demos, copying bot work to user message demo, and removing node from all components except table)
Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>
@rebeccaalpert
rebeccaalpert force-pushed the footnotes branch 2 times, most recently from 3360bc8 to 8a11eca Compare September 5, 2025 13:38

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall I think this is looking great. Just a couple of comments:

  • Personally still unsure about the background color change when clicking a footnote, but I'm fine keeping it and us keeping an eye/ear out for feedback on it. Mainly worried of it being a visual affordance we don't really employ elsewhere (or at least I haven't come across it I guess, wouldnt be surprised if it's common outside core React). And if this isn't something we have (easy) control over that's understandable as well
  • The second "back to" button in the user messages example is fairly wide compared to the Bot Message example:
    image
  • The id's on the footnote headers in both examples are the same so that should be resolved if we can easily do that right now. Otherwise we can open a followup if it'd be a little more involved

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're gonna open follow ups as needed for the above, otherwise this lgtm!

@rebeccaalpert

Copy link
Copy Markdown
Member Author

See follow-ons:

We'll adjust the highlighting if/when products start using footnotes to match what's actually being done. No one's actually using these yet and we just wanted to demo some sort of interaction even if it's not necessarily 100% perfect.

@rebeccaalpert
rebeccaalpert merged commit 82496f7 into patternfly:main Sep 5, 2025
7 checks passed
@github-actions

github-actions Bot commented Sep 5, 2025

Copy link
Copy Markdown

🎉 This PR is included in version 6.4.0-prerelease.20 🎉

The release is available on:

Your semantic-release bot 📦🚀

rebeccaalpert added a commit to rebeccaalpert/virtual-assistant that referenced this pull request Oct 24, 2025
Styled footnotes and added demos with some onClick events.

Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>

Assisted-by: Cursor (used for debugging demos, copying bot work to user message demo, and removing node from all components except table)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for footnotes

5 participants