Code block annotations

[glitteringkatie]

Hi! We have a request from docsmobile to build a code block that supports annotations.

Asciidoc

We need to replace the annotation functionality in asciidoc (they call it code callouts, but functionally they are annotations).

The existing asciidoc notation and UI is the following:

[source,js]
-----
let happy = false <1>
const donut = 1
if (donut > 1) happy = true <2>
console.log(`I am ${!happy && 'not '}happy`)
-----
<1> By default, `happy` is set to `false`.
<2> But, if you have at least one https://www.doritedonuts.com/[`donut`] a day, set `happy` to `true`.

Requirements

At a minimum, we need a way to:

• Specify which lines to add "callouts" (think numbered footnotes) and author notes for each callout
• In the code block, render a "callout" icon next to the line code
• Below the code block, render notes next to the corresponding "callout" icon
• One note can relate to multiple lines (example below)

```js
const something = 3 [^1]
const another = something * 4 
console.log(another) [^1]
```
[^1]: Here's a note

As far as "variables" for the notes, we have no need to support anything fancier than just numbers (i.e. 1 vs aNote)

Proposal

I started looking into how we could implement this only because I wasn't sure if we would hack it or get you all to help (thanks for helping!). Here are my thoughts based on a little dig into code, you all are the EUI experts!

Component

EUI already has line-specific styling. It looks like this:

I think we could write a partner to highlightByLine that instead of adding line styling, it adds a notification badge that we'll treat as the annotation footnote number.

Then there would be a new partner component called EuiCodeNotes that lists all the notification badges (as footnotes) and the annotation it should accompany. I don't think this component should be exposed.

The main new thing would be a wrapper of the two: EuiAnnotatedCode or something. It would take an array of tuples (or object, whatever) that would include the line number paired with the note. It would then pass the list of line numbers to annotate to the EUI Code Block and the list of notes to the EuiCodeNotes component.

Additional context

For the original request and any other notes that you might find interesting, see our issue: https://github.com/elastic/docsmobile/issues/337.

It's worth noting that part of the request is to be able to go from MDX -> this component. That means figuring out what our MDX syntax should/could be and maybe writing a remark parser. I see that as something in our court but if that's an interesting thing for EUI, I'm open to talking about that!

[cee-chen]

[Just a suggestion/starting a brainstorming thread]

Rather than footnotes, what are your thoughts a beacon + popover trigger of some sort over the line number? (Codepen link with source)

I was using EuiTour to very quickly sketch out the concept, but I would not propose actually using it for the final implementation - you'd probably want to use underlying EuiPopover/EuiWrappingPopover and some kind of way to portal a beacon toggle to the line number (can be selected via [data-line-number]), but more customization of the popover itself.

I suggest this approach because:

1. Footnotes can be tricky for screen reader users, you need some way of clicking from the number to the footnote; otherwise it's not accessible
2. For very vertically tall code blocks, jumping up and down to connect the footnote to the originating line is tedious and unnecessary cognitive overhead (vs clicking a popover toggle and viewing the note in-place without losing your scroll position)
3. Portaling a beacon overlay will cause significantly less layout disruption and is more predictable than attempting to put a footnote number at the end of a line
4. This one is a bit selfish, but with this approach, you could potentially use just the EuiPortal, EuiBeacon, and EuiPopover as-is to build your annotation component that sits on top of EuiCodeBlock without needing to modify EuiCodeBlock directly. To be honest, scope creep and feature support has become an issue for the EUI team as of late, and the more complex and single-use-case a feature request becomes, the more leery I am of implementing it directly into EUI (and having to support & potentially expand it in the future). EuiCodeBlock is already one of the more technically complex and challenging components to read, understand, maintain, and test, and I'm not 100% sold on adding this functionality for just one team.

Thoughts? @miukimiu @daveyholler

[cee-chen]

Ahhh, sorry, I think I once again missed the mark 😅 If it's the first use case, I'd much rather consumers do something like

const repeatedNote = <>Some text that applies to multiple single lines</>;

<EuiCodeBlock
  lineNumbers={{
    start: 1,
    highlight: '1, 3-5, 10',
    annotations: {
      3: repeatedNote,
      5: 'Not a repeated note',
      7: repeatedNote
    },
  }}
/>

[colleenmcginnis]

Is it more common that folks will use one note to cover multiple lines in a "this note pertains to this line of code and also this other line of code" way or in a "this note pertains to this chunk of continuous code"? In my head, it was the first use case.

Yes it's the first use case. I think both could be valuable hypothetically, but if we're talking about docsmobile/EUI as a replacement for AsciiDoc, the first use case is the critical one since AsciiDoc allows you to reuse the same note on multiple individual lines (that are not necessarily continuous), but doesn't provide a way to attach a note to a "chunk of continuous code".

[glitteringkatie]

@cee-chen the repeatedNote solution makes sense to me!

[cee-chen]

Alright, thanks y'all! I'm moving ahead with the simpler Record<number, ReactNode> data structure for the annotations API in that case! 🏃

[glitteringkatie]

Yay! Thank you!

[elizabetdev]

Hi @glitteringkatie,

FYI we had a very similar discussion started by Gareth one year ago: #5609.

I like @cee-chen suggestion but I found the beacon very distracting. In cases where we can have two or more beacons, the animations seems to me too much. We could of course disable the animation, but then the beacon wouldn't look clickable.

So based on @cee-chen initial idea, we could introduce a new type of beacon for annotations with an "i" icon or maybe a "question mark". It could also accept numbers and be clickable. Right now the EuiBeacon is not clickable.

Also in #5609, Dave suggested the following.

gULXwpUfVVyvf-eU.mp4

But it my opinion the left content can take too much space and it only makes sense for long content. For small one.. Not sure.

@cee-chen and @glitteringkatie, let me know your thoughts on these ideas.

[cee-chen]

Thank you for the much more polished mockup @miukimiu!! 🙌 I love it.

Dave's example is neat, but I agree it's not clear that it will scale well with all use cases, whereas I think the i icon/beacon + popovers is significantly easier to handle an unknown amount of content.

[glitteringkatie]

Sounds like we're in agreement here--Dave's example takes up a ton of space and I feel like is more ideal if you're telling a story with the code as opposed to pointing out a little note.

[cee-chen]

PR with implementation and staging link: #6580