Skip to content

Home table update #174

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.

Sign up for GitHub

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

Merged
kavithacm merged 19 commits into from
Nov 3, 2021
Merged

Home table update #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9bd5b7e
Create history_table.tsx
kavithacm Oct 18, 2021
97b09cb
Update history_table.tsx
kavithacm Oct 18, 2021
78513c4
Update home.tsx
kavithacm Oct 18, 2021
276a590
Update history_table.tsx
kavithacm Oct 21, 2021
693b261
Update history_table.tsx
kavithacm Oct 21, 2021
ba84e8d
Update home.tsx
kavithacm Oct 26, 2021
f6cd878
Update home.tsx
kavithacm Oct 29, 2021
e7783f8
Delete home.tsx
kavithacm Oct 29, 2021
e4d33d3
merge (#3)
kavithacm Oct 29, 2021
6487a5a
Merge branch 'opensearch-project:main' into main
kavithacm Oct 29, 2021
e1437d3
Merge branch 'opensearch-project:main' into main
kavithacm Oct 29, 2021
a7266dc
Merge branch 'opensearch-project:main' into main
kavithacm Nov 2, 2021
a38237b
Update history_table.tsx
kavithacm Nov 2, 2021
0955eed
Update home.tsx
kavithacm Nov 2, 2021
ac28702
Delete public/components/explorer/home_table directory
kavithacm Nov 2, 2021
d3d611f
Merge branch 'opensearch-project:main' into home_table_update
kavithacm Nov 2, 2021
a63dbcf
Update explorer.ts
kavithacm Nov 2, 2021
b228f5c
Update history_table.tsx
kavithacm Nov 2, 2021
bda2115
Update history_table.tsx
kavithacm Nov 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dashboards-observability/common/constants/explorer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const TAB_EVENT_TITLE = 'Events';
export const TAB_EVENT_ID_TXT_PFX = 'main-content-events-';
export const TAB_CHART_ID_TXT_PFX = 'main-content-vis-';
export const HAS_SAVED_TIMESTAMP = 'hasSavedTimestamp';
export const FILTER_OPTIONS = ['Visualization', 'Query'];

export const DATE_PICKER_FORMAT = 'YYYY-MM-DD HH:mm:ss';
export const TIME_INTERVAL_OPTIONS = [
Expand Down
2 changes: 1 addition & 1 deletion dashboards-observability/public/components/explorer/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export const Home = (props: IHomeProps) => {
wrapText={ true }
>
<EuiTitle size="s">
<h1>{ "Saved Queries and Visualizations" }</h1>
<h1>{ "Queries and Visualizations" }</h1>
</EuiTitle>
<EuiSpacer size="s" />
<Table savedHistory={savedHistories}
Expand Down
111 changes: 83 additions & 28 deletions dashboards-observability/public/components/explorer/home_table/history_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@
* GitHub history for details.
*/

import React, { useState, useRef, useEffect } from 'react';

import {
EuiBasicTable,
EuiSpacer,
EuiLink,
} from '@elastic/eui';
import React, { useState, useRef } from 'react';

import { EuiSpacer, EuiLink, EuiInMemoryTable, EuiIcon, EuiLoadingChart } from '@elastic/eui';
import { FILTER_OPTIONS } from '../../../../common/constants/explorer';

interface TableData {
savedHistory: [];
savedQuerySearch: (searchQuery: string, selectedDateRange: [], selectedTimeStamp, selectedFields: []) => void;
savedQuerySearch: (
searchQuery: string,
selectedDateRange: [],
selectedTimeStamp,
selectedFields: []
) => void;
}

export function Table(options: TableData) {
Expand All @@ -31,7 +32,6 @@ export function Table(options: TableData) {
const pageSizeRef = useRef();
pageSizeRef.current = pageSize;


const onTableChange = ({ page = {} }) => {
const { index: pageIndex, size: pageSize } = page;

Expand All @@ -40,44 +40,100 @@ export function Table(options: TableData) {
};

const columns = [
{
field: 'type',
name: '',
Copy link
Member

@ps48 ps48 Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add name as type here so that it's visible on the table.

Copy link
Collaborator Author

@kavithacm kavithacm Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ani wanted the icon column name to be empty and there's a separate column named type too for the type of the query

width: '3%',
Copy link
Member

@joshuali925 joshuali925 Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does a percentage width work well for an icon? could you try zoom in and zoom out the page to see if it still looks good

Copy link
Collaborator Author

@kavithacm kavithacm Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the width is for the distance between two columns. I tried zoom in and out, still looks good. I have attached an image for reference
new view
.

render: (item) => {
if (item == 'Visualization') {
return (
<div>
<EuiLoadingChart size="m" />
Copy link
Member

@ps48 ps48 Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This can be visualizeApp Icon.

Copy link
Collaborator Author

@kavithacm kavithacm Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was Ani's choice to have a chart as icon for visualization, that's why went with this

Copy link
Member

@joshuali925 joshuali925 Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is specifically used for loading, is there a icon for this?

Copy link
Collaborator Author

@kavithacm kavithacm Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it was Ani's choice to have a chart as icon for visualization

</div>
);
} else {
return (
<div>
<EuiIcon type="discoverApp" size="m" />
</div>
);
}
},
},
{
field: 'data',
name: 'Name',
render: (item)=>{return <EuiLink onClick={() =>
{options.savedQuerySearch(item.query, [item.date_start, item.date_end], item.timestamp, item.fields)}}>
{item.name}
</EuiLink>},
align: 'left',
render: (item) => {
return (
<EuiLink
onClick={() => {
options.savedQuerySearch(
Copy link
Collaborator

@mengweieric mengweieric Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a different name? the name savedQuerySearch is a little bit confusing, since it is just a function dispatching data.

Copy link
Collaborator Author

@kavithacm kavithacm Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually dispatching data for the query search, thats why i used the same name. should i still change it?

Copy link
Collaborator

@mengweieric mengweieric Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's fine, the dispatch will be removed once the changes to support passing object id gets merged.

item.query,
[item.date_start, item.date_end],
item.timestamp,
item.fields
);
}}
>
{item.name}
</EuiLink>
);
},
},
{
field: 'description',
name: 'Description',
field: 'type',
name: 'Type',
align: 'right',
},
];



let queryType = '';
const queries = options.savedHistory.map((h) => {
const savedObject = h.hasOwnProperty('savedVisualization')
Copy link
Collaborator

@mengweieric mengweieric Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you need to check if it is savedVisualization more than once, you could do just const isSavedVisualization = h.hasOwnProperty('savedVisualization'), and then use isSavedVisualization in the following code.

Copy link
Collaborator Author

@kavithacm kavithacm Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm checking if its savedVisualization or not for each object, so it may differ for each entry of data. as i need to display the type of the query as a column, i need to populate it

Copy link
Collaborator

@mengweieric mengweieric Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean in your each loop.

Copy link
Collaborator Author

@kavithacm kavithacm Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

? h.savedVisualization
: h.savedQuery;
const isSavedVisualization = h.hasOwnProperty('savedVisualization');
if (isSavedVisualization) {
queryType = 'Visualization';
} else {
queryType = 'Query';
}
return {
data: {
name: savedObject.name,
query: savedObject.query,
date_start: savedObject.selected_date_range.start,
date_end : savedObject.selected_date_range.end,
timestamp: savedObject.selected_timestamp?.name || '',
fields: savedObject.selected_fields?.tokens || []
date_end: savedObject.selected_date_range.end,
timestamp: savedObject.selected_timestamp?.name,
fields: savedObject.selected_fields?.tokens || [],
},
name: savedObject.name || '',
description: savedObject.description || '',

name: savedObject.name,
type: queryType,
};
});


const totalItemCount = queries.length;

const search = {
box: {
incremental: true,
},
filters: [
{
type: 'field_value_selection',
field: 'type',
name: 'Type',
multiSelect: false,
options: FILTER_OPTIONS.map((i) => ({
value: i,
name: i,
view: i,
})),
},
],
};

const pagination = {
pageIndex,
pageSize,
Expand All @@ -88,14 +144,13 @@ export function Table(options: TableData) {
return (
<div>
<EuiSpacer size="xl" />
<EuiBasicTable
items={queries.slice(pageIndex * pageSize, pageIndex * pageSize + pageSize)}
<EuiInMemoryTable
Copy link
Member

@joshuali925 joshuali925 Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using InMemoryTable requires storing all items in memory instead of a constant 10 items (which is pageSize, although I see it was not implemented previously either). will there be a performance issue when number of queries is large?

Copy link
Collaborator Author

@kavithacm kavithacm Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about that part, i'll have a talk with Eric regarding this

items={queries}
columns={columns}
pagination={pagination}
onChange={onTableChange}
search={search}
/>
</div>
);
}