[Runtime fields editor] Form UI#81766
[Runtime fields editor] Form UI#81766sebelga merged 18 commits intoelastic:feature/runtime-field-editorfrom
Conversation
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
|
@elasticmachine merge upstream |
mistic
left a comment
There was a problem hiding this comment.
Changes in files under operations team code owners LGTM
|
Thanks for the review @mistic ! Do you know why the build is failing? Not sure I understand the error: |
|
@elasticmachine merge upstream |
|
@sebelga the problem is that baselines for each commit to the |
|
Exploring a solution in #81938, mind giving me a few hours to see how feasible this solution is? |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
|
#81938 changed the CI metrics check to only fail if the PR is merging into a tracked branch, like master. The If a similar error was to occur on a branch targeting master then the report would say it was failed. |
|
Awesome, thanks for looking into this @spalger ! |
alisonelizabeth
left a comment
There was a problem hiding this comment.
Great work @sebelga! It's exciting to start seeing this take shape. I left a few comments, nothing blocking. Testing locally and worked as expected.
|
|
||
|
|
||
| |{kib-repo}blob/{branch}/x-pack/plugins/runtime_fields[runtimeFields] | ||
| |WARNING: Missing README. |
There was a problem hiding this comment.
Can we add a readme? I understand it might not contain much info at this point and is subject to change.
There was a problem hiding this comment.
I have the README.md ready in my following PR 😊
| <EuiFlexGroup justifyContent="flexEnd"> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiLink | ||
| href={`${docsBaseUri}/to-be-defined`} |
There was a problem hiding this comment.
I think I'd prefer to comment the link component out for now, rather than merge it with an invalid href.
There was a problem hiding this comment.
I have updated the href in my following PR to point correctly to https://www.elastic.co/guide/en/elasticsearch/painless/master/painless-lang-spec.html
| fullWidth | ||
| > | ||
| <CodeEditor | ||
| languageId="painless" |
There was a problem hiding this comment.
You should be able to import the language id from kbn-monaco now.
import { PainlessLang } from '@kbn-monaco';
<CodeEditor
languageId={PainlessLang.ID}
....
/>
There was a problem hiding this comment.
Good to know, I will update it. Although IMO it should be the CodeEditor that should have the languageId correctly typed with available options ('painless'|'other')
| return ( | ||
| <> | ||
| <EuiFormRow label={label} fullWidth> | ||
| <EuiComboBox |
There was a problem hiding this comment.
I'm not as familiar with the proposed design - but would the EuiSelect suffice here, as there are only 6 supported types?
There was a problem hiding this comment.
For now, I've mirrored what CJ did for the mappings editor "runtime" field type in #79940
I'm happy to revisit this later if we don't think the ComboBox brings value 👍
|
Thanks for the review @alisonelizabeth ! I've addressed your comments in my following PR |
This PR contains the UI component of the runtime field editor form. It is a presentational component that wraps the form with the 3 required fields:
Interface
The component has the following interface
The component is not meant to be consumed directly, there will be another PR with the "container" component that will act as a layer between the apps consuming the runtime field editor and this UI component.
I've also made the x-pack "runtime_fields" plugin a bundle dependency of the "index_management" plugin and imported from there the constants and TS type required for the mappings editor "runtime" field type.
Note: The flyout, EuiCodeBlock and "Update" button are not part of what is about to be merged. This PR is only the UI form component for the runtime field.
How to test
Replace the content of the
x-pack/plugins/index_management/public/application/sections/home/home.tsxfile with the code below and navigate to the "index management" app.