-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Add theme and more lexer rules #178757
[Console] Add theme and more lexer rules #178757
Conversation
/ci |
1 similar comment
/ci |
Pinging @elastic/kibana-management (Team:Kibana Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vis changes look good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @yuliacech! I tested locally and the highlighting works as expected. Code changes also LGTM.
I left a small comment about the colors but it's not that important IMO - just something to think about.
const stringTextColor = '#009926'; | ||
const commentTextColor = '#4C886B'; | ||
const variableTextColor = '#0079A5'; | ||
const booleanTextColor = '#585CF6'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering whether we could use the already defined euiTheme
colors for consistency, like it is done for the code editor theme (which I think is used in Painless lab). For example, the method color could be euiTheme.euiColorAccentText
. But this decision depends on whether we want to keep the theme as close to the ace editor theme as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, the hard coded values are not great, but we can't currently map them one-to-one without changing the colors. I think there are 2 ways to improve this code: either re-use the already existing json code editor theme, but that changes the look of console. Or alternatively, we try to keep the original look but re-use the built-in colors from EUI as much as possible.
I added a section to #176926 with follow up work once the initial language definition is implemented.
3d1b6a6
to
f58da35
Compare
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
Summary
This PR adds a theme for the Console language in Monaco editor and adds more lexer rules to bring the highlighting of the input closed to the original in Ace editor.
Screenshots
Monaco editor
Ace editor
How to test
console.dev.enableMonaco: true
to `kibana.dev.yml``xjson
highlighting still worksa. Navigate to Ingest pipelines and click the "create from csv" button
b. Load a valid csv file, for example this one
Known issues that will be addressed in follow up PRs
xjson
and Console (added to the follow up list in [Console Monaco Migration] Implement language definition #176926)