Add deep link page number to task history page.#1801
Conversation
|
It looks like this doesn't automatically update the query param when the user clicks through the pagination buttons. We should add that so that these links become discoverable. |
| sortDirection: defaultSortDirection, | ||
| sortTime: null, | ||
| chunkNum: 1, | ||
| chunkNum: initialPageNumber || 1, |
There was a problem hiding this comment.
This could also be done with default props.
| {deleted || ( | ||
| <TaskHistoryTable | ||
| requestId={requestId} | ||
| onPageChange={num => router.replace(`${location.pathname}?taskHistoryPage=${num}`)} |
There was a problem hiding this comment.
This works. You could also pass in a location object with a query prop for react-router. That might be a little safer in terms of ensuring that you don't override any other existing query parameters or the location hash.
|
|
||
| componentDidMount() { | ||
| const { requestId, initialPageNumber } = this.props; | ||
| if (initialPageNumber) { |
There was a problem hiding this comment.
How is the data loaded if there is no initialPageNumber? The code for both cases should probably be in a single place.
…askHistoryTable's componentDidMount now that RequestDetailPage now passes the component a default initialPageNumber. Pass router.replace a location object in onPageChage prop for TaskHistoryTable.
andyhuang91
left a comment
andyhuang91
left a comment
There was a problem hiding this comment.
Looks like there's a merge conflict now. Aside from that, LGTM
|
|
||
| componentDidMount() { | ||
| const { requestId, initialPageNumber } = this.props; | ||
| this.setState({ loading: true }); |
There was a problem hiding this comment.
This could be set as part of the initial state in the constructor
|
🚢 |
3 participants
No description provided.