Change UI of history page to table form using material-table #71
Conversation
| @@ -47,7 +47,7 @@ export default function AlertsList(props) { | |||
| disableTypography | |||
| primary={<Typography variant="body1" >Alert on | |||
| <Box fontWeight='fontWeightBold' display='inline' m={1} style={{ color: '#0FA3B1'}}> | |||
| {` ${allAlerts.get(alertId).timestampDate} (${formatDate(allAlerts.get(alertId).timestampDate)})`} | |||
| {` ${allAlerts.get(alertId).timestampDate} (${formatDate(allAlerts.get(alertId).timestampDate, true)})`} | |||
There was a problem hiding this comment.
nit: i don't think its good passing in boolean to a function, since it is less readable and arbitrary what the boolean stands for. (paula mentioned this before in one of my PRs, not sure if completely applicable to js code thou)
or if a boolean is necessary, maybe you can do:
formatDate(allAlerts.get(alertId).timestampDate, /** mFirst = */ true)
There was a problem hiding this comment.
that's a good point. I'll add the comment
| import Search from '@material-ui/icons/Search'; | ||
| import ViewColumn from '@material-ui/icons/ViewColumn'; | ||
|
|
||
| export const tableIcons = { |
There was a problem hiding this comment.
no worries, if it's too much effort!
|
here's a GIF showing the export as CSV / export as PDF: |
Could you add this by editing the PR description so its more visible :) |
| import { formatDate } from '../time_utils'; | ||
|
|
||
| const createData = (id, timestampDate, numAnomalies, status, priority, metrics, dimensions) => { |
There was a problem hiding this comment.
so createData basically just creates the object with those parameters so its in the right format for the table?
| filtering: true | ||
| }} | ||
| title="" | ||
| exportFileName="BlackSwan_Alerts" |
There was a problem hiding this comment.
btw wha thappens if you export multiple files consecutively since theyre the same filename?
There was a problem hiding this comment.
it'll be the same name just with (1) or (2) (it's what the file system does for naming)
| render: rowData => rowData.dimensions.split(DATA_DELIMITER).map((dimension) => | ||
| <Chip label={dimension} color="primary" variant="outlined" | ||
| size="small" style={styles.chip} | ||
| />) | ||
| } |
There was a problem hiding this comment.
-
currently there's only a single dimension chip and a single metric chip for each row right? was curious on how this ends up getting displayed on the page with multiple chips (on the UI side -- not really a coding thing).
-
also since metrics and dimensions both have similar logic creating chips from the rowData might be useful to separate this out as a separate thing.
There was a problem hiding this comment.
Yeah so when I get the alert, I put the metrics + dimensions each all in one string (to allow for searching) with the delimiter (%). Then when the data is being rendered, I split on the delimiter and create chip for each metric / dimension.
| />) | ||
| } | ||
| ]} | ||
| data={rows(allAlerts)} |
There was a problem hiding this comment.
so the history component isn't doing any fetching of data right? it just loads all of the stuff that's passed in , but doesnt do any actual calls?
and in addition, user arent able to do any form of editing (except for frontend-based filtering/sorting) based on the currently visualized/seen alerts (but not other alerts that are in the backend but might not be displayed in this current page for whatever reason), is my understanding correct?
There was a problem hiding this comment.
Yeah they can't edit the alerts on this page. You can actually allow it to be edited and I don't think it would be too difficult to include it, but I also felt that because being able to edit the alert's properties is already available on the alert's page and the Alerts page, wouldn't need to have it on the table as well.
Change information representation on history page to table format using https://material-table.com/#/.
Format of table:
UI screenshot:
here's a GIF showing the export as CSV / export as PDF:
