-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Discover] Deangularization of search embeddable #100552
Changes from 25 commits
6478960
c796fdf
b7bbfb3
3ad0532
7b943fc
f6dd986
1ec4052
9dcb235
abdd8e4
ebf0f77
d5f5a43
36179f0
c59638f
90ad57a
0f7edfd
b434f8e
e1f9e03
9cdf842
5840f60
b7ecfaa
2e9a9c8
a1cb95f
d334657
04a3cfa
3bf5eb5
2603f3d
c540629
69e7646
99548a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,86 @@ | ||||||||||
/* | ||||||||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||||||||||
* or more contributor license agreements. Licensed under the Elastic License | ||||||||||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||||||||||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||||||||||
* Side Public License, v 1. | ||||||||||
*/ | ||||||||||
|
||||||||||
import { render } from 'react-dom'; | ||||||||||
import React, { useRef, useEffect } from 'react'; | ||||||||||
import { I18nProvider } from '@kbn/i18n/react'; | ||||||||||
import { IScope } from 'angular'; | ||||||||||
import { getServices } from '../../../kibana_services'; | ||||||||||
import { DocTableLegacyProps, injectAngularElement } from './create_doc_table_react'; | ||||||||||
|
||||||||||
type AngularEmbeddableScope = IScope & { renderProps?: DocTableEmbeddableProps }; | ||||||||||
|
||||||||||
export interface DocTableEmbeddableProps extends Partial<DocTableLegacyProps> { | ||||||||||
refs: HTMLElement; | ||||||||||
} | ||||||||||
|
||||||||||
function getRenderFn(domNode: Element, props: DocTableEmbeddableProps) { | ||||||||||
const directive = { | ||||||||||
template: `<doc-table | ||||||||||
class="panel-content" | ||||||||||
columns="renderProps.columns" | ||||||||||
data-description="{{renderProps.searchDescription}}" | ||||||||||
data-shared-item | ||||||||||
data-test-subj="embeddedSavedSearchDocTable" | ||||||||||
data-title="{{renderProps.sharedItemTitle}}" | ||||||||||
filter="renderProps.onFilter" | ||||||||||
hits="renderProps.rows" | ||||||||||
index-pattern="renderProps.indexPattern" | ||||||||||
is-loading="renderProps.isLoading" | ||||||||||
on-add-column="renderProps.onAddColumn" | ||||||||||
on-change-sort-order="renderProps.onSort" | ||||||||||
on-move-column="renderProps.onMoveColumn" | ||||||||||
on-remove-column="renderProps.onRemoveColumn" | ||||||||||
render-complete | ||||||||||
sorting="renderProps.sort" | ||||||||||
total-hit-count="renderProps.totalHitCount" | ||||||||||
use-new-fields-api="renderProps.useNewFieldsApi"></doc-table>`, | ||||||||||
}; | ||||||||||
|
||||||||||
return async () => { | ||||||||||
try { | ||||||||||
const injector = await getServices().getEmbeddableInjector(); | ||||||||||
return await injectAngularElement(domNode, directive.template, props, injector); | ||||||||||
} catch (e) { | ||||||||||
render(<div>error</div>, domNode); | ||||||||||
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 assume we expect this to never happen. Since we still catch it, I think we should render a more meaningful message, that will help us trace this in case it would ever happen. If we expect it to "NEVER" happen, I think alternatively we might just not catch the error at all, and instead let it be handled by the overall embeddable error infrastructure. |
||||||||||
} | ||||||||||
}; | ||||||||||
} | ||||||||||
|
||||||||||
export function DiscoverDocTableEmbeddable(props: DocTableEmbeddableProps) { | ||||||||||
return ( | ||||||||||
<I18nProvider> | ||||||||||
<DocTableLegacyInner {...props} /> | ||||||||||
</I18nProvider> | ||||||||||
); | ||||||||||
} | ||||||||||
|
||||||||||
function DocTableLegacyInner(renderProps: DocTableEmbeddableProps) { | ||||||||||
const scope = useRef<AngularEmbeddableScope | undefined>(); | ||||||||||
|
||||||||||
useEffect(() => { | ||||||||||
if (renderProps.refs && !scope.current) { | ||||||||||
const fn = getRenderFn(renderProps.refs, renderProps); | ||||||||||
fn().then((newScope) => { | ||||||||||
scope.current = newScope; | ||||||||||
}); | ||||||||||
} else if (scope?.current) { | ||||||||||
scope.current.renderProps = { ...renderProps }; | ||||||||||
scope.current.$applyAsync(); | ||||||||||
} | ||||||||||
}, [renderProps]); | ||||||||||
|
||||||||||
useEffect(() => { | ||||||||||
return () => { | ||||||||||
if (scope.current) { | ||||||||||
scope.current.$destroy(); | ||||||||||
} | ||||||||||
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.
Suggested change
|
||||||||||
}; | ||||||||||
}, []); | ||||||||||
return <React.Fragment />; | ||||||||||
} |
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.
🎨 Since this object has only one property, and doesn't seem to be used anywhere except for accessing that single property, maybe we could just make this a
const template
instead.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.
For consistency with other angular directives and overall readability, I'd like to keep it as is