[EuiCodeBlock] Support different highlight and annotations colours

[machadoum]

Is your feature request related to a problem? Please describe. We want to highlight lines with warnings and errors on a CSV file uploader.

Describe the solution you'd like Allow EuiCodeBlock component lineNumbers.highlight and lineNumbers.annotations colour customization.

Describe alternatives you've considered We don't need advanced colour customization for our use case, so it would be fine if EuiCodeBlock only allows us to set the colour from a small subset, such as 'info' | 'warning' | 'error'.

Desired timeline Our goal is to deliver the feature on April 16, 2024. Given the short notice, we may need to simplify the design.

[tkajtoch]

I chatted with @machadoum about this feature and they're moving with a simpler implementation at the moment to get it done by Kibana 8.14. We have more time to work on the details

[JasonStoltz]

@tkajtoch Please let us know if this is still a valid request or if we can close this.

[JasonStoltz]

Let's try to move the conversation to this thread if possible for visibility!

[tkajtoch]

Hey @r4zr32d3k1l would you mind sharing more details from the design perspective on this feature request?

[machadoum]

This is still under development, but at the moment, we are displaying the info icon in blue:

It would look better if we could customize the colour.

[JasonStoltz]

We chatted about this last week. One thing that was top of mind for us is figuring out how to maintain color contrast -- prism supports a wide variety of languages with variations in syntax highlighting, so we'd need to make sure we're using colors that work well across the board.

Other aspects of this seem simpler -- like adding the ability to configure which icons are shown for annotations, which seems like it might improve the user experience for you. I wonder if that alone would be enough to accommodate your use case.

The other thing we weigh here is how generally useful a feature. Do you think there are use cases for this outside of this one particular use case?

[JasonStoltz]

@r4zr32d3k1l @machadoum Any other thoughts on this?

If you have some concrete ideas to make this more tangible we would happy to pursue this further, but as it is now we'll need some help making this a more concrete feature request.

[r4zr32d3k1l]

@JasonStoltz I'd prefer to stick with the existing EUI colors that we're currently using for warning and error messages. We shouldn't introduce anything new. If you're concerned about contrast, we should address this issue within our design system and resolve it globally.

Regarding usability, having the ability to display code is beneficial, and it’s always good to have the capability to show errors if we can detect them. So yes, I'm hopeful that we can identify other use cases where this feature could be applied.

[JasonStoltz]

@r4zr32d3k1l,

Thanks for that response.

FWIW, I perceive the hardest part of this ask is selecting color values that work well regardless code text color. Would it be possible to simplify this ask by just supporting just the ability to color annotations?

For comparison:

Also, we're past 8.14 and 8.15 is quickly approaching. Do you have an updated target for this? I'm not sure how quickly we'll be able to prioritize this, so I'd appreciate your guidance on the urgency here.

FWIW, if we're not able to get to this quick enough for you, we would appreciate and gladly accept a PR for it.

I'm thinking out loud about the effort here:

Our API currently looks like this:

      highlight: '32, 34-37, 40',
      annotations: {
        34: 'Coordinates can be obtained from Elastic Maps',
        38: (
          <>
            Allowed types: <EuiCode>Point</EuiCode>, <EuiCode>Area</EuiCode>
          </>
        ),
      },

I think that regardless of the route we take, we'll need a more robust API, maybe something like this:

      highlight: [
        { 
          lineStart: 32,
          color: 'danger' // default to 'primary'
        }, {
          lineStart: 34,
          lineEnd: 37, //optional
          color: 'warning'
        },
        {
          lineStart: 40
        },
      annotations: {
        34: {
          description: 'Coordinates can be obtained from Elastic Maps',
          color: 'danger' // default to 'primary'
        },
        38: {
          description: 'Coordinates can be obtained from Elastic Maps'
        }
      ],

As far as I can tell, there's not a whole lot of technical complexity to actually implementing this change.

If we end up with concerns with contrast, I think we could just message in our docs as guidance.

[JasonStoltz]

Following up here, the requesting team has implemented the following work around, separating invalid and valid lines into separate codeblocks.

For now, we're deprioritizing this work.