Adds Pagination to Data Table #1367
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
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
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 | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,118 @@ | ||||||
| import { FormControl, MenuItem, Select } from "@material-ui/core"; | ||||||
| import * as React from "react"; | ||||||
| import styled from "styled-components"; | ||||||
| import Button from "./Button"; | ||||||
| import Flex from "./Flex"; | ||||||
| import Icon, { IconType } from "./Icon"; | ||||||
| import Spacer from "./Spacer"; | ||||||
| import Text from "./Text"; | ||||||
|
|
||||||
| export interface Props { | ||||||
| /** CSS MUI Overrides or other styling. */ | ||||||
| className?: string; | ||||||
| /** func for forward one page button */ | ||||||
| onForward: () => void; | ||||||
| /** func for skip to last page button */ | ||||||
| onSkipForward: () => void; | ||||||
| /** func for back one page button */ | ||||||
| onBack: () => void; | ||||||
| /** func for skip to start button */ | ||||||
| onSkipBack: () => void; | ||||||
| /** onChange func for perPage select */ | ||||||
| onSelect: (value) => void; | ||||||
|
There was a problem hiding this comment. Let's give this There was a problem hiding this comment. it's an event, and it needs to always be an event right? Is there a way to express that in Typescript? There was a problem hiding this comment. Something like
Suggested change
You can also just leave it blank if you don't expect the user to do anything with the arg. |
||||||
| /** options for perPage select */ | ||||||
| perPageOptions?: number[]; | ||||||
| /** starting index */ | ||||||
| index: number; | ||||||
| /** total rows */ | ||||||
| length: number; | ||||||
| /** all objects */ | ||||||
| totalObjects: number; | ||||||
| } | ||||||
|
|
||||||
| function unstyledPagination({ | ||||||
| className, | ||||||
| onForward, | ||||||
| onSkipForward, | ||||||
| onBack, | ||||||
| onSkipBack, | ||||||
| onSelect, | ||||||
| perPageOptions = [25, 50, 75, 100], | ||||||
| index, | ||||||
| length, | ||||||
| totalObjects, | ||||||
| }: Props) { | ||||||
| return ( | ||||||
| <Flex wide align end className={className}> | ||||||
| <FormControl> | ||||||
| <Flex align> | ||||||
| <label htmlFor="pagination">Rows Per Page: </label> | ||||||
| <Spacer padding="xxs" /> | ||||||
| <Select | ||||||
| id="pagination" | ||||||
| variant="outlined" | ||||||
| defaultValue={perPageOptions[0]} | ||||||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => { | ||||||
| onSelect(e.target.value); | ||||||
| }} | ||||||
| > | ||||||
| {perPageOptions.map((option, index) => { | ||||||
| return ( | ||||||
| <MenuItem key={index} value={option}> | ||||||
| {option} | ||||||
| </MenuItem> | ||||||
| ); | ||||||
| })} | ||||||
| </Select> | ||||||
| </Flex> | ||||||
| </FormControl> | ||||||
| <Spacer padding="base" /> | ||||||
| <Text> | ||||||
| {index + 1} - {index + length} out of {totalObjects} | ||||||
| </Text> | ||||||
| <Spacer padding="base" /> | ||||||
| <Flex> | ||||||
| <Button | ||||||
| color="inherit" | ||||||
| variant="text" | ||||||
| aria-label="skip to first page" | ||||||
| disabled={index === 0} | ||||||
| onClick={() => onSkipBack()} | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| > | ||||||
| <Icon type={IconType.SkipPreviousIcon} size="medium" /> | ||||||
| </Button> | ||||||
| <Button | ||||||
| color="inherit" | ||||||
| variant="text" | ||||||
| aria-label="back one page" | ||||||
| disabled={index === 0} | ||||||
| onClick={() => onBack()} | ||||||
| > | ||||||
| <Icon type={IconType.NavigateBeforeIcon} size="medium" /> | ||||||
| </Button> | ||||||
| <Button | ||||||
| color="inherit" | ||||||
| variant="text" | ||||||
| aria-label="forward one page" | ||||||
| disabled={index + length >= totalObjects} | ||||||
| onClick={() => onForward()} | ||||||
| > | ||||||
| <Icon type={IconType.NavigateNextIcon} size="medium" /> | ||||||
| </Button> | ||||||
| <Button | ||||||
| color="inherit" | ||||||
| variant="text" | ||||||
| aria-label="skip to last page" | ||||||
| disabled={index + length >= totalObjects} | ||||||
| onClick={() => onSkipForward()} | ||||||
| > | ||||||
| <Icon type={IconType.SkipNextIcon} size="medium" /> | ||||||
| </Button> | ||||||
| </Flex> | ||||||
| </Flex> | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| export const Pagination = styled(unstyledPagination)``; | ||||||
|
|
||||||
| export default Pagination; | ||||||
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
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.
There shouldn't be a
labelanddisplayLabelprop here;labelis already mean to give control over what gets displayed.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.
For the _.sort function to work the labels have to be all lowercase to match the fields on the rows which meant of lot of string.toLowerCase()-ing which I h8ed.
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.
I would go with
string.toLowerCase()myself. As it is in this PR, it is a leaky abstraction, and you are asking the user to think about how the sorting logic works.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.
The only other thing that's coming to mind is if someone wants to sort columns with a label that won't match when it's shifted to lowercase. Like "Last Commit" with
last_commitas the key. It makes those columns not sortable right?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.
You can do stuff like
_.lowerCase(_.snakeCase(string))if you want to normalize the values. I would have to see the full filtering implementation to say for sure.