This repository has been archived by the owner on Mar 23, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 44
Bug 1520857- add support in tc-web for displaying information from the lastFire table #413
Open
Biboswan
wants to merge
14
commits into
taskcluster:master
Choose a base branch
from
Biboswan:listLastFires
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+288
−93
Open
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
c57174f
Initial display of lastfire fires
Biboswan 7a33df8
Improve responsiveness and overall ui with Grid
Biboswan 5b5c97d
Remove unwanted class
Biboswan a154a50
Merge two hook graphql queries into one
Biboswan f55678d
Replace expansion panel with datatable
Biboswan c5f5a16
Add pagination
Biboswan 87993e3
minor fix
Biboswan f98d385
Add toggle view for error
Biboswan 4674382
Add drawer to display error info
Biboswan 0156365
Minor tweak
Biboswan e238f6f
add link icon for taskids
Biboswan 8bb249f
minor tweak
Biboswan 2b28062
Remove 1-20 part and text rename
Biboswan aa51ef9
add scrollbar
Biboswan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,14 +7,27 @@ import { | |
oneOf, | ||
oneOfType, | ||
object, | ||
bool, | ||
} from 'prop-types'; | ||
import { withStyles } from '@material-ui/core/styles'; | ||
import Table from '@material-ui/core/Table'; | ||
import TableBody from '@material-ui/core/TableBody'; | ||
import TableFooter from '@material-ui/core/TableFooter'; | ||
import TableCell from '@material-ui/core/TableCell'; | ||
import TableHead from '@material-ui/core/TableHead'; | ||
import TableSortLabel from '@material-ui/core/TableSortLabel'; | ||
import TableRow from '@material-ui/core/TableRow'; | ||
import TablePagination from '@material-ui/core/TablePagination'; | ||
|
||
@withStyles(() => ({ | ||
noDisplay: { | ||
display: 'none', | ||
}, | ||
tablePaginationRoot: { | ||
position: 'relative', | ||
left: '207%', | ||
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 don't think we need this. You can look at how they do it in https://material-ui.com/demos/tables/. |
||
}, | ||
})) | ||
/** | ||
* A table to display a set of data elements. | ||
*/ | ||
|
@@ -26,6 +39,11 @@ export default class DataTable extends Component { | |
sortByHeader: null, | ||
sortDirection: 'desc', | ||
noItemsMessage: 'No items for this page.', | ||
isPaginate: false, | ||
}; | ||
|
||
state = { | ||
page: 0, | ||
}; | ||
|
||
static propTypes = { | ||
|
@@ -66,6 +84,10 @@ export default class DataTable extends Component { | |
* A message to display when there is no items to display. | ||
*/ | ||
noItemsMessage: string, | ||
/** | ||
* Whether to paginate the table | ||
*/ | ||
isPaginate: bool, | ||
}; | ||
|
||
handleHeaderClick = ({ target }) => { | ||
|
@@ -76,17 +98,25 @@ export default class DataTable extends Component { | |
} | ||
}; | ||
|
||
handleChangePage = (event, page) => { | ||
this.setState({ page }); | ||
}; | ||
|
||
render() { | ||
const { | ||
classes, | ||
items, | ||
columnsSize, | ||
renderRow, | ||
headers, | ||
sortByHeader, | ||
sortDirection, | ||
noItemsMessage, | ||
isPaginate, | ||
} = this.props; | ||
const colSpan = columnsSize || (headers && headers.length) || 0; | ||
const { page } = this.state; | ||
const rowsPerPage = 5; | ||
|
||
return ( | ||
<Table> | ||
|
@@ -115,9 +145,36 @@ export default class DataTable extends Component { | |
</TableCell> | ||
</TableRow> | ||
) : ( | ||
items.map(renderRow) | ||
items | ||
.slice(page * rowsPerPage, page * rowsPerPage + rowsPerPage) | ||
.map(renderRow) | ||
)} | ||
</TableBody> | ||
{isPaginate && ( | ||
<TableFooter> | ||
<TableRow> | ||
<TablePagination | ||
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. We should:
|
||
classes={{ | ||
root: classes.tablePaginationRoot, | ||
spacer: classes.noDisplay, | ||
caption: classes.noDisplay, | ||
}} | ||
component="div" | ||
count={items.length} | ||
rowsPerPage={rowsPerPage} | ||
rowsPerPageOptions={[rowsPerPage]} | ||
page={page} | ||
backIconButtonProps={{ | ||
'aria-label': 'Previous Page', | ||
}} | ||
nextIconButtonProps={{ | ||
'aria-label': 'Next Page', | ||
}} | ||
onChangePage={this.handleChangePage} | ||
/> | ||
</TableRow> | ||
</TableFooter> | ||
)} | ||
</Table> | ||
); | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 are we adding pagination? Is the last fire endpoint paginated (i.e., uses continuation token)?
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.
As per discussion with Dustin in irc we are keeping as many as 100 lastfire info rows for each hookId (expiration logic). Hence I think showing all rows at once won't be good.
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.
We also only purge those once per day, so a particularly active hook might have many more rows than that at some time during the day!