feat/developer search paginate #13
Conversation
| </TableCell> | ||
| </TableRow> | ||
| <TablePagination |
There was a problem hiding this comment.
Don't we already have a Pagination component? Use that instead. Don't rewrite a new component that does the same thing
| @@ -12,9 +12,12 @@ import { | |||
| TableContainer, | |||
| TableHead, | |||
| TableRow, | |||
| OutlinedInput, | |||
| TablePagination, | |||
| } from "@mui/material"; | |||
| import EditIcon from "@mui/icons-material/Edit"; | |||
| import React from "react"; | |||
There was a problem hiding this comment.
I think can just import what you need like ChangeEvent, no need React.ChangeEvent
| const handleEditClick = (id: number) => { | ||
| handleEdit(id, type); | ||
| }; | ||
|
|
||
| //filter data based on query |
There was a problem hiding this comment.
No need comment for self-evident code, just rename the function to something clearer like filterContent or something
| @@ -65,7 +128,10 @@ export const TabContent = ({ | |||
| </TableRow> | |||
| ))} | |||
| </TableBody> | |||
|
|
|||
There was a problem hiding this comment.
remember to use yarn format before pushing
SebassNoob
force-pushed
the
feat/developer-search-paginate
branch
from
66be574
to
6a00261
Compare
| interface PaginationProps { | ||
| pageInfo: { page: number; size: number; total: number; pages: number }; | ||
| handlePageChange: (newPage: number) => void; | ||
| HStackStyles?: HStackStyles; |
There was a problem hiding this comment.
added this optional parameter to customise Hstack properties
There was a problem hiding this comment.
I think no need ba, just straight away pass it in, this one abit verbose since you can just pass into HStack straight
There was a problem hiding this comment.
im not sure what you mean, but i dont think i can pass styles into Hstack directly via the pagination component. hence the reasoning for this addition
| return validData; | ||
| }; | ||
|
|
||
| //paginate data into chunks |
| <TableCell colSpan={2}> | ||
| <ChakraProvider> | ||
| <Pagination | ||
| // page is 0 indexed, but Pagination component is 1 indexed |
| interface HStackStyles { | ||
| spacing?: number; | ||
| mt?: string; | ||
| } |
There was a problem hiding this comment.
ok nvm, i think is some inconsistent styling, usually it should have at least some space in between different code component
There was a problem hiding this comment.
remove this new interface, remove HStackStyles from PaginationProps, Allow Pagination to take in new prop styles
| const debouncedHandlePageChange = debounce(handlePageChange, 100); | ||
|
|
||
| return ( | ||
| <HStack spacing={4} justifyContent="center" mt="10%"> | ||
| <HStack | ||
| spacing={HStackStyles?.spacing ? HStackStyles.spacing : 4} |
There was a problem hiding this comment.
do we need a new data structure just to do spacing for hstacks? just declare the values by itself here lo
There was a problem hiding this comment.
i just dont want to break anything, so previous implementations have a default value of 4
There was a problem hiding this comment.
yea you can just pass in directly and do a conditional for default spacing 4 if spacing doesn't exist here, no need for new interface just to pass this in
also make sure that when you change screen components this way, it doesn't mess up the mobile design because its supposed to be responsive
There was a problem hiding this comment.
pagination should be consistent throughout, just keep it as 4
| @@ -12,9 +12,13 @@ import { | |||
| TableContainer, | |||
| TableHead, | |||
| TableRow, | |||
| OutlinedInput, | |||
| TablePagination, | |||
| } from "@mui/material"; | ||
| import EditIcon from "@mui/icons-material/Edit"; | ||
| import React from "react"; | ||
| import React, { useState, ChangeEvent, MouseEvent } from "react"; |
| <HStack | ||
| spacing={HStackStyles?.spacing ? HStackStyles.spacing : 4} | ||
| justifyContent="center" | ||
| mt={HStackStyles?.mt ? HStackStyles.mt : "10%"} |
There was a problem hiding this comment.
after adding styles prop just change to mt={styles?.mt ? styles.mt : "10%"}
imo this 10% warrants a review but just do ^ first
| handlePageChange={(newPage: number) => { | ||
| setPage(newPage - 1); | ||
| }} | ||
| HStackStyles={{ spacing: 10, mt: "0%" }} |
There was a problem hiding this comment.
can styles={{ mt: "0%" }} after everything above
|
other than the comments everything else lgtm chakra needs to be migrated to mui but that can be done another time when we do a clean up after we merge the new FE in |
fix/cleanup editmodal
new and improve FE (NEED TO CLEAN UP)
…nn/holy-grail into fix/remove-chakra-admin
fix/remove chakra admin
…ichannnnn/holy-grail into feat/developer-search-paginate
This PR adds search and pagination to the /developer page.
This PR may cause a merge conflict with #12 if both are approved, which I will fix if a conflict is present :).