-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Use EuiCodeBlock for JSON settings, and overflow if long value #20744
Conversation
<EuiCode> | ||
default_value | ||
</EuiCode> | ||
<UNDEFINED> |
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.
UNDEFINED
in place of React.Fragment
is a bug which is fixed with jest >= v23.0.0. I'm investigating a possible jest upgrade in another branch.
💚 Build Succeeded |
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.
Code LGTM, just had a question about the language.
<Fragment> | ||
Default: | ||
<EuiCodeBlock | ||
language="js" |
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.
should this be json instead of js?
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.
good call, for some reason I thought both would highlight the same, but json
is a little cleaner. updated.
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.
LGTM! Checked code and tested locally.
💚 Build Succeeded |
Fixes #20156
In Advanced Settings, change display of JSON default setting value to use
EuiCodeBlock
, and use itsoverflowHeight
prop for long values. Screenshot shows a long JSON default value and a short JSON value: