-
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
[Lens] Embeddable react refactor #186642
[Lens] Embeddable react refactor #186642
Changes from 231 commits
542b751
6a68e2d
4649991
81591fb
c34d4c2
1bcaa20
e49d957
15ebfb4
e8ae231
b27c92d
5b1f902
8e72a8b
8ff30ef
3e35668
3a29e3d
3f82a8d
7d4b0f0
cdc027f
67757a8
654a3b1
9f284f2
e174bfa
daace28
b2e0e60
054a1e4
fa27a42
d9c13f4
740ff3a
1c5a8d5
393adf2
d23dec7
77a8e29
5c8babb
ae3a6bb
ae6532d
8d8a204
a5a02cf
1b986bd
9bbcbc0
8b8d4df
87bf399
20d4da6
5bf0d0b
16c1460
381b50a
08462d7
ea89ca9
e09f66c
d142bca
41c4b7d
e5529e0
e0bd89b
9281c8b
c0f047c
8c7b2de
ad11067
83ee184
ee63ebd
f35c2e8
2a6e744
b7936fb
86c6e99
9fcb65e
86b0ed4
c627d97
d408c52
d927f0f
7e4465e
9e7cb04
1b540ab
e4a6a0f
2333cce
0044ff4
91f000e
ab10de9
f32dd32
a0a7e57
b3a04e0
2858bea
0406df7
2c56102
f7f2db6
2cd7164
1f81099
49c3da3
9005870
eecab21
8b1bb37
adc41be
67b8bea
db1abef
09ea73e
80cc2e9
c8194bc
3cf9dd7
c8e45a1
2a7089c
3db0cec
38d2110
93a8ca1
1ce6773
8601b91
3fb36ae
649236f
feaee2b
67f9996
7807a9c
efae4d5
818c7c9
204a1b3
a9d94c0
9e3af25
cd890c5
6a77f58
de4e768
6f2e467
e1d4025
e1ecc53
b35e7e0
e4ffbdc
14b7390
c881fba
49ed84d
4ea7622
f387938
9a150e3
6139715
0210d30
d8dd05d
ff942a7
05fae9d
f675390
de2e3ea
ed079f4
9f6dee4
e495aee
23b59a7
b46f639
bc083d7
9fdac9e
20a389a
456173f
1fd093c
35749d2
48c34a1
9854d66
dcc2baa
7efe070
90d9e43
8fdec4e
3a4100f
1264bac
b801e99
33b0235
f546409
53e5729
846d753
55cda92
7cb51e9
f6167c5
42bb70e
dc8a110
09b6163
e27d471
8e81678
11f3164
13f57c2
6b487d4
2140e3f
37760dd
61d3e1a
dbab16d
de7c04e
a80f618
d86ddad
b307ed1
9c2629a
99f5528
fdfec0d
11579e7
2a20a17
6d21cec
aaf1f31
463ee58
2f246fe
2dcb2eb
c9f86ae
3b982d9
c7c6de0
0bab982
a362bd7
ce07080
a8f49af
5ebf31f
ab4409b
c9ee526
7527b64
14e0aa5
b2be257
f3f5f40
058a373
1b2b134
3c4fedb
0b8d1f8
a1ac7eb
3d4e6b2
a785186
1c25c87
602d3f1
7485734
16b4a59
409cbf1
06c715d
7aacc18
218b84f
8ce8be2
5386438
5560171
9ae6fc2
2a38177
1578d17
fe503fb
647bfe5
0a3f07c
348c2d5
0b67223
f355a68
233ef76
e86c510
ef33089
6f62aab
cffbb97
ae797d8
2f7f49d
163c65b
53f6e81
a1690a8
2b4dfa8
28c65b9
323fd01
95399ea
bf670f6
5758fb4
6605240
0e65ba7
5fcf51c
d0a48b1
27e27bf
a6ffd5c
684b106
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
dej611 marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
stratoula marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,14 +20,14 @@ import { | |
EmbeddableComponent, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something is being malformed when it comes to saved object state for by value objects. On this PR, I created this dashboard (3 by value panels, 1 by reference, one with filter, one with query): and then I ran this saved object in main (or switch branches), I get a bunch of errors in the console: Is this intentional? Should anything from saved object persisted state change? It makes the potential revert risky so we should keep an eye on it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm currently investigating this bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
FieldBasedIndexPatternColumn, | ||
TypedLensByValueInput, | ||
LensByValueInput, | ||
} from '@kbn/lens-plugin/public'; | ||
import { Datatable } from '@kbn/expressions-plugin/common'; | ||
import { render, screen, waitFor } from '@testing-library/react'; | ||
import '@testing-library/jest-dom'; | ||
import userEvent from '@testing-library/user-event'; | ||
import { I18nProvider } from '@kbn/i18n-react'; | ||
import { GroupPreview } from './group_preview'; | ||
import { LensByValueInput } from '@kbn/lens-plugin/public/embeddable'; | ||
import { DATA_LAYER_ID, DATE_HISTOGRAM_COLUMN_ID, getCurrentTimeField } from './lens_attributes'; | ||
import { EuiSuperDatePickerTestHarness } from '@kbn/test-eui-helpers'; | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After the refactor the new Embeddable system won't adapt its height to its container, so the wrapper There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new custom component didn't get the right |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was required to match the new type for the Inspector. |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbondyra |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More @mbondyra |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before the embeddable used to push the entire An additional change was around the SaveModal component: before there was some confusion around the |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No more usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was not able to reproduce #184600 when testing this PR. Let's try unskiping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's unskip this after this PR get merged. I just don't want to keep adding changes to this PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't validate this PR fully if our main test suit for Discover visualizations is skipped. To me it's a blocker for merging this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've unskipped the test, let's see what the CI says. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been unskipped and it is passing as 6f62aab . |
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.
Just import move.