-
Notifications
You must be signed in to change notification settings - Fork 100
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
Feature/visualization #83
Changes from 14 commits
d2eb3c8
6a4f97a
fd7a466
e4958a9
ab4f6dd
0cf3882
0d82d03
b034479
e080853
10bcf39
5101c15
2fa5ab3
355e767
3cfe933
ddf8551
74b6b56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,18 +49,12 @@ export interface IExplorerTabFields { | |
|
||
export interface IExplorerFields { | ||
[SELECTED_FIELDS]: Array<IField>; | ||
[UNSELECTED_FIELDS]: Array<IField> | ||
[UNSELECTED_FIELDS]: Array<IField>; | ||
} | ||
|
||
export interface IExplorerProps { | ||
tabId: string; | ||
query: any; | ||
explorerData: any; | ||
explorerFields: any; | ||
setSearchQuery: (query: string, tabId: string) => void; | ||
querySearch: (tabId: string) => void; | ||
addField: (field: IField, tabId: string) => void; | ||
removeField: (field: IField, tabId: string) => void | ||
pplService: any; | ||
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 type should just be 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. Yes, missed this one. |
||
} | ||
|
||
export interface LogExplorer { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
* | ||
* Modifications Copyright OpenSearch Contributors. See | ||
* GitHub history for details. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { mountWithIntl as mount } from 'test_utils/enzyme_helpers'; | ||
import { debouncedComponent } from './debounced_component'; | ||
import { act } from 'react-dom/test-utils'; | ||
|
||
describe('debouncedComponent', () => { | ||
test('immediately renders', () => { | ||
const TestComponent = debouncedComponent(({ title }: { title: string }) => { | ||
return <h1>{title}</h1>; | ||
}); | ||
expect(mount(<TestComponent title="hoi" />).html()).toMatchInlineSnapshot(`"<h1>hoi</h1>"`); | ||
}); | ||
|
||
test('debounces changes', async () => { | ||
const TestComponent = debouncedComponent(({ title }: { title: string }) => { | ||
return <h1>{title}</h1>; | ||
}, 1); | ||
const component = mount(<TestComponent title="there" />); | ||
component.setProps({ title: 'yall' }); | ||
expect(component.text()).toEqual('there'); | ||
await act(async () => { | ||
await new Promise((r) => setTimeout(r, 1)); | ||
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. not very familiar with the event loop, but would it be possible that this finishes before debounced component re-renders and test fails? since the delays are both 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. To me it looks like it would not fail since when setting the props, this action would be put into the event loop, then the following assertion statement would run immediately and pass. No matter what statement is actually picked by the main thread to run next (setting props to 'yall' or await act...), it would actually end up making sure the debounce component has changed its value to 'yall', before the last assertion runs. Since event handlers will be invoked in the same order that they were bound in. 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. got it, looks good then |
||
}); | ||
expect(component.text()).toEqual('yall'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
* | ||
* Modifications Copyright OpenSearch Contributors. See | ||
* GitHub history for details. | ||
*/ | ||
|
||
import React, { useState, useMemo, useEffect, memo, FunctionComponent } from 'react'; | ||
import { debounce } from 'lodash'; | ||
|
||
/** | ||
* debouncedComponent wraps the specified React component, returning a component which | ||
* only renders once there is a pause in props changes for at least `delay` milliseconds. | ||
* During the debounce phase, it will return the previously rendered value. | ||
*/ | ||
export function debouncedComponent<TProps>(component: FunctionComponent<TProps>, delay = 256) { | ||
const MemoizedComponent = (memo(component) as unknown) as FunctionComponent<TProps>; | ||
|
||
return (props: TProps) => { | ||
const [cachedProps, setCachedProps] = useState(props); | ||
const debouncePropsChange = useMemo(() => debounce(setCachedProps, delay), [setCachedProps]); | ||
|
||
// cancel debounced prop change if component has been unmounted in the meantime | ||
useEffect(() => () => debouncePropsChange.cancel(), [debouncePropsChange]); | ||
debouncePropsChange(props); | ||
|
||
return React.createElement(MemoizedComponent, cachedProps); | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
* | ||
* Modifications Copyright OpenSearch Contributors. See | ||
* GitHub history for details. | ||
*/ | ||
|
||
export * from './debounced_component'; | ||
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. Can we avoid the wild card import usage here? 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. Ok, will change all of them |
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.
could you remove this block, also add some scripts (mainly
osd
andbuild
), reference here https://github.com/opensearch-project/notifications/blob/develop/dashboards-notifications/package.json#L12-L20There 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.
Sure, will do.