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

[EuiCodeBlock] Support different highlight and annotations colours #7629

Open
machadoum opened this issue Mar 28, 2024 · 9 comments
Open

[EuiCodeBlock] Support different highlight and annotations colours #7629

machadoum opened this issue Mar 28, 2024 · 9 comments

Comments

@machadoum
Copy link
Member

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

Screenshot 2024-03-28 at 11 03 55

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 tkajtoch self-assigned this Mar 28, 2024
@tkajtoch
Copy link
Member

tkajtoch commented Apr 4, 2024

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
Copy link
Member

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

@JasonStoltz
Copy link
Member

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

@tkajtoch
Copy link
Member

tkajtoch commented Apr 8, 2024

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

@machadoum
Copy link
Member Author

This is still under development, but at the moment, we are displaying the info icon in blue:
Screenshot 2024-04-09 at 17 39 02

It would look better if we could customize the colour.

@JasonStoltz
Copy link
Member

JasonStoltz commented Apr 9, 2024

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
Copy link
Member

JasonStoltz commented Apr 29, 2024

@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
Copy link

@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
Copy link
Member

@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:
Screenshot 2024-05-20 at 3 12 32 PM

Screenshot 2024-05-20 at 3 13 39 PM

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants