-
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
Add support of Data View switching for Agg-Based visualizations #132184
Conversation
3dda5ff
to
fbf8341
Compare
@elasticmachine merge upstream |
really like the behavior. the only thing that's weird now is the duplication of the data view name. I see why it's nice to have it over on the right (that's where it used to be too) but it does make the edit action feel out of place a bit. just minor feedback - removing this is also a bit strange when it comes for needing a place to unlink the saved search so I think this PR could still be merged without a solution to the double data view name issue. as a complete aside this makes me yearn for the collapsable field list in Lens too :D (and the broader unification of discover & lens' field lists someday) |
Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors) |
@elasticmachine merge upstream |
LGTM!, code review only |
@elasticmachine merge upstream |
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.
Looks mostly good, some small nits
@@ -381,6 +382,12 @@ export class VisualizeEmbeddable | |||
this.subscriptions.push(this.handler.loading$.subscribe(this.onContainerLoading)); | |||
this.subscriptions.push(this.handler.render$.subscribe(this.onContainerRender)); | |||
|
|||
this.subscriptions.push( |
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.
Why this change?
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 need to show error in EmbeddableVis
. Without this, we will not render anything, it will just be a white screen
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.
How did it work before?
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.
Ok... I've reproduced field error on the previous implementation. It's not working. User see loading indicator
Screen.Recording.2022-05-17.at.3.16.57.PM.mov
@@ -48,7 +48,7 @@ const createEditorStateReducer = | |||
} | |||
|
|||
case EditorStateActionTypes.DISCARD_CHANGES: { | |||
return initEditorState(action.payload); | |||
return initEditorState(action.payload.vis); |
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.
Why this change?
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 expanded payload in the previous solution, then it turned out to be unnecessary, but it seems to me that passing as an object is more correct and I decided to leave it like this.
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.
Plus I see now all actions looks the same, all payloads wrapped into object.
let stateUpdatesSubscription: Subscription; | ||
|
||
if (appState && visInstance) { | ||
const syncDataView = async ({ dataView }: VisualizeAppState, setDirty = true) => { |
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.
unused flag?
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.
right, thanks!
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @alexwizp |
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, works as expected!
…tic#132184) * Add support of Data View switching for Agg-Based visualizations * fix CI * add use_date_view_updates * implement sync with state * cleanup * cleanup * cleanup * Update index.ts * fix PR comments * Update use_data_view_updates.ts * Update use_data_view_updates.ts Co-authored-by: Kibana Machine <[email protected]>
* wip I * add alert table state in case * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * add new API to get FeatureID form registrationContext and update UI to use this new API * rm dead code * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * remove unnecessary memo * adds tests for case view helpers * Move http call to API and add tests for getFeatureIds * fix type + unit test * add unit tests + cleanup * add new api integration test for _feature_ids * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * Fix small type creating typescript slowness * remove console log * use import type for validfeatureId * force any to improve typescript performance * Update APM (#132270) Co-authored-by: Renovate Bot <[email protected]> * [ResponseOps][Docs] Updating ServiceNow docs with OAuth setup instructions (#131344) * Updating ServiceNow docs. Need screenshots * Adding screenshots * Fix nested screenshots and lists * Tweaks and screenshots * Updates * blergh * Apply suggestions from code review Co-authored-by: Lisa Cawley <[email protected]> * Apply suggestions from code review Co-authored-by: Mike Côté <[email protected]> Co-authored-by: lcawl <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Mike Côté <[email protected]> * Show polling options when 'Data streams' option is selected in the Console Settings modal. (#132277) * [Osquery] Make Osquery All with All base privillege (#130523) * [XY] Add normalizeTable function to correct works with esdocs (#131917) * Add normalizeTable function to correct works with esdocs * Fix types * Fix types * Fix CI * Fix CI * Some fixes * Remove fallback with min/max value for domain * Added tests * Some refactoring Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Yaroslav Kuznietsov <[email protected]> * [Osquery] Add default osquery_saved_query objects (#129461) * [Unified Search] Show error message for invalid date filter value (#131290) * feat: added show error message for invalid date * refact: move logic in HOC * feat: refactoring code and added translation * refact show error * refact: show error message * refact: remove translation * refactor: changed menu for show FilterEdit * fix: open/close popover * feat: field.type => KBN_FIELD_TYPES * feat: remove extra code with with input check and refactored filter item * feat: added tests and refactoring code * refact: getFieldValidityAndErrorMessage * feat: return isInvalid checking in valur input type for string, ip * Update navigation landing pages to use appLinks config (#132027) * Update navigation landing pages to use appLinks config * Please code review * align app links changes * Update links descriptions * Rollback title changes * Fix wrong links descriptions * Fix unit tests * Fix description Co-authored-by: semd <[email protected]> * [Cloud Posture] add resource findings page flyout (#132243) * [Discover] Add a tour for Document Explorer (#131125) * [Discover] Add "Take a tour" button to the Document Explorer callout * [Discover] Tmp * [Discover] Add a first Document Explorer tour step * [Discover] Add other Document Explorer tour steps * [Discover] Update tour steps positioning * [Discover] Add gifs to tour steps * [Discover] Refactor how tour steps are registered * [Discover] Add new step to the tour. Update tour steps text. * [Discover] Improve steps positioning * [Discover] Fix positioning for Add field step * [Discover] Add icons to tour steps * [Discover] Reorganize components * [Discover] Skip Columns step when it's not available * [Discover] Rename components * [Discover] Add some tests * [Discover] Fix positioning * [Discover] Fix props * [Discover] Render steps only if the tour is active * [Discover] Update gifs * [Discover] Add image alt text for gifs * [Discover] Tag the Take tour button * [Discover] Update text and tests * [Discover] Add more tests * [Discover] Rename assets directory * [Discover] Fix tour in mobile view. Improve steps positioning and animation. * [Discover] Update text in tour steps * [Discover] Update sort.gif * [Discover] Update image width * Update src/plugins/discover/public/components/discover_tour/discover_tour_provider.tsx Co-authored-by: gchaps <[email protected]> * Update src/plugins/discover/public/components/discover_tour/discover_tour_provider.tsx Co-authored-by: gchaps <[email protected]> * [Discover] Update sort.gif * [Discover] Fix code style Co-authored-by: gchaps <[email protected]> Co-authored-by: Kibana Machine <[email protected]> * [XY] Add `minTimeBarInterval` arg (#128726) * Added `xAxisInterval` arg * Add validation * Add tests * Rename xAxisInterval to minTimeBarInterval and add validation * Fix imports * Add tests to validation * Fix conflicts * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * Fix tests Co-authored-by: Kibana Machine <[email protected]> * do not use barrel imports * do not use barrel import * do not use barrel import * do not use barrel imports * do not use barrel import * import types * Add tests * Fix cases bundle size * Add more tests * [Fleet] Add new API to get current upgrades (#132276) * Add support of Data View switching for Agg-Based visualizations (#132184) * Add support of Data View switching for Agg-Based visualizations * fix CI * add use_date_view_updates * implement sync with state * cleanup * cleanup * cleanup * Update index.ts * fix PR comments * Update use_data_view_updates.ts * Update use_data_view_updates.ts Co-authored-by: Kibana Machine <[email protected]> * [Security Solution] Responsive styling fixes (#131951) * [Discover] Add Analytics No Data Page (#131965) * [Discover] Add Analytics No Data Page * Make showEmptyPrompt parameter optional * Remove unused import * Remove unnecessary test * Fix test * Update failing test? * Update failing test * Changing the order of functional tests * Fix error handling * Addressing PR comments Co-authored-by: Kibana Machine <[email protected]> * Remove barrel export from public index file * remove barrel export * Re-export missing exports * Turn off feature flag Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Esteban Beltran <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Renovate Bot <[email protected]> Co-authored-by: Ying Mao <[email protected]> Co-authored-by: lcawl <[email protected]> Co-authored-by: Mike Côté <[email protected]> Co-authored-by: CJ Cenizal <[email protected]> Co-authored-by: Tomasz Ciecierski <[email protected]> Co-authored-by: Uladzislau Lasitsa <[email protected]> Co-authored-by: Yaroslav Kuznietsov <[email protected]> Co-authored-by: Nodir Latipov <[email protected]> Co-authored-by: Pablo Machado <[email protected]> Co-authored-by: semd <[email protected]> Co-authored-by: Or Ouziel <[email protected]> Co-authored-by: Julia Rechkunova <[email protected]> Co-authored-by: gchaps <[email protected]> Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Nicolas Chaulet <[email protected]> Co-authored-by: Alexey Antonov <[email protected]> Co-authored-by: Steph Milovic <[email protected]> Co-authored-by: Maja Grubic <[email protected]>
Summary
This PL adds the ability to change the
Data View
for already saved classic visualizations. These changes are related to theUnified Search
initiative and continue to implement the overall design in line with other plugins (Lens, Discover, etc.)Screens
Screen.Recording.2022-05-13.at.12.08.50.PM.mov