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

[Console][Monaco] Fix low contrast colors in selected text on Manoco Console #186778

Closed

Conversation

sakurai-youhei
Copy link
Member

@sakurai-youhei sakurai-youhei commented Jun 23, 2024

Summary

This PR changes the background colors to increase the contrast of neighboring colors on the Manoco Console.

Fixes #186772

Colors in this PR:
image
image

Colors before this PR:
image
image

Colors on 8.4.1:
image
image

Checklist

  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
  • This was checked for cross-browser compatibility

For maintainers

@sakurai-youhei sakurai-youhei added bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature release_note:skip Skip the PR/issue when compiling release notes labels Jun 23, 2024
@sakurai-youhei sakurai-youhei marked this pull request as ready for review June 23, 2024 12:22
@sakurai-youhei sakurai-youhei requested review from a team as code owners June 23, 2024 12:22
@sakurai-youhei
Copy link
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.2MB 3.2MB +102.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@MichaelMarcialis
Copy link
Contributor

@ryankeairns: Do the color changes being proposed here align with the colors you're proposing for the ES|QL Monaco editor? If not, should they be for consistency?

@ryankeairns
Copy link
Contributor

@ryankeairns: Do the color changes being proposed here align with the colors you're proposing for the ES|QL Monaco editor? If not, should they be for consistency?

For the ES|QL editor, I believe we leave the selection background color to your system default... although, that can produce some not great contrast, too. This feels a bit more full proof and could be done for ES|QL too - I'll track that.

As for the background colors - we used EUI theme variables over there, perhaps that can/should be done here too?
Referring to the color settings here: https://github.com/elastic/kibana/blob/main/packages/kbn-monaco/src/esql/lib/esql_theme.ts

@sakurai-youhei
Copy link
Member Author

I'm closing this PR in favor of #189060

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Console Dev Tools Console Feature release_note:skip Skip the PR/issue when compiling release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Console][Monaco] No contrast between selected text and current line colors
6 participants