Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Response Ops] [Rule Form] Add Rule Form Flyout v2 #206685
base: main
Are you sure you want to change the base?
[Response Ops] [Rule Form] Add Rule Form Flyout v2 #206685
Changes from 93 commits
1ef801c
1faf112
4cb5b1d
e5b287a
74f59b7
dd6e167
6dd013d
69eb8fa
74d5339
646af0f
d4c91d3
2d61517
1ffea47
19f2fdc
3e9ebe4
70d4dab
9c4d831
547c4da
c2174d6
4719faf
51e33f9
c25c283
dc8c9ab
114e98a
e3a0e7e
dbc9712
fd7abec
4dbc4f0
b0f977b
4b806ac
13429c1
f557eb2
286721f
1fd34d0
c437919
dbbe03a
f9f82ed
f21566b
0efa925
fb70192
4fd1228
f2e01b9
e6befaf
07ee45a
370a2b6
908d36c
52c6414
aa0287f
9ef99b5
bb09a7d
d182820
118a154
b8a2e33
09d456d
c89a707
565c974
def9107
3f16b58
27cb8b3
688c145
c3eff4f
3ef3181
756cfa8
95b14da
aa0abd0
990a73c
c4a2c9b
3772d57
319f869
99c4dba
9298807
0b991bf
732ce23
4539f97
929ce73
e9948a2
944621a
699300b
1f1f1a5
89f8aaa
0249c2d
0a01b81
42e12ee
abb8326
1013d20
4453609
a8ffcf1
619a37b
850bb6b
acaddb9
3ab44d2
fc004b0
c1edbf3
2d229e6
30c3d46
f8b000b
75f6b91
ce5d6d6
7797540
887b65d
cee9258
c72958b
9e7716d
314d14c
e1fac6f
3b0f4aa
35acdb7
feb0c83
8b13750
9c5b11a
b168e01
30b89de
f4aff0f
6e765ec
c015311
85971c3
587f5c2
5e84003
fd7e080
9e3d7e9
20c0821
78cf8db
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shouldn't we call
props.onCancel
either way? For example: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.
Shouldn't also translate it?
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.
aria-labelledBy
takes a DOMid
, not a stringThere 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.
Thanks, I was not aware.
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.
There is a service to dynamically load ECS data for selected fields: https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/fields_metadata/README.md
Or when rendering it's possible to simply use
<FieldDescription .../>
component from https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-field-utils/src/components/field_description/field_description.tsxThere 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.
will this be addressed in this PR?
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.
If it helps for this use case, can be definitely done separately.
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've honestly had to switch this back and forth from dynamic import to static import like 3 times over the past couple months of making changes to
alerts-ui-shared
and the rule form package, everything has been unrelated and just moving other code around, so I haven't had time to actually examine what EcsFlat is used for here. Onmain
Webpack puts it in an asnyc chunk and on this PR for some reason it changes its mind about that. Handling this better is beyond the scope of this PR, I'm just trying to keep the bundle size down so it can merge.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 will open an issue for this. There are other places where we load the ECS package.
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.
Issue: #211585.