Skip to content

Commit

Permalink
MMT-3561: Addressing Comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mandyparson committed Feb 2, 2024
1 parent a6e54e2 commit 868cbae
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 36 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"bootstrap": "^5.3.2",
"classnames": "^2.3.2",
"commafy": "^0.0.6",
"commafy-anything": "^3.0.4",
"compact-object-deep": "^1.0.0",
"cookie": "^0.6.0",
"esbuild": "^0.19.5",
Expand Down
1 change: 0 additions & 1 deletion static.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"cmrHost": "http://localhost:4000",
"version": "development",
"jwtSecret": "jwt_secret"

},
"ummVersions": {
"ummC": "1.17.3",
Expand Down
6 changes: 3 additions & 3 deletions static/src/js/components/DraftList/DraftList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ const DraftList = ({ draftType }) => {
limit
})
const { count, items = [] } = drafts
console.log(count)

const noDraftsError = `No ${draftType} drafts exist for the provider ${providerId}`
const noDataError = `No ${draftType} drafts exist for the provider ${providerId}`

const [downloadDraft] = useLazyQuery(DOWNLOAD_DRAFT, {
onCompleted: (getDraftData) => {
Expand Down Expand Up @@ -161,7 +160,8 @@ const DraftList = ({ draftType }) => {
classNames={['col-md-4', 'col-md-4', 'col-auto', 'col-auto']}
loading={loading}
data={data}
error={noDraftsError}
error={error}
noDataError={noDataError}
count={count}
setOffset={setOffset}
limit={limit}
Expand Down
11 changes: 4 additions & 7 deletions static/src/js/components/Pagination/Pagination.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import Col from 'react-bootstrap/Col'
const PaginationComponent = ({
limit,
count,
setoffset
setOffset
}) => {
const [pageNum, setPageNum] = useState(1)

Expand All @@ -28,7 +28,7 @@ const PaginationComponent = ({

const handleItemClick = (currentPage) => {
setPageNum(currentPage)
setoffset((currentPage - 1) * limit)
setOffset((currentPage - 1) * limit)
}

const generatePaginationItems = () => {
Expand Down Expand Up @@ -106,22 +106,19 @@ const PaginationComponent = ({
const newCurrentPage = pageNum + direction

setPageNum(newCurrentPage)
setoffset((newCurrentPage - 1) * limit)
setOffset((newCurrentPage - 1) * limit)
}

return (
<Row>
<Col xs="auto">
{/* <h1>Pagination</h1> */}
<div className="mx-auto">
<Pagination>
<Pagination.Prev
disabled={pageNum === 1}
onClick={() => handlePageChange(-1)}
/>

{generatePaginationItems()}

<Pagination.Next
onClick={() => handlePageChange(1)}
disabled={pageNum >= lastPageNum}
Expand All @@ -138,7 +135,7 @@ PaginationComponent.defaultProps = {
}

PaginationComponent.propTypes = {
setoffset: PropTypes.func.isRequired,
setOffset: PropTypes.func.isRequired,
limit: PropTypes.number.isRequired,
count: PropTypes.number
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import userEvent from '@testing-library/user-event'

import PaginationComponent from '../Pagination'

const setoffset = jest.fn()
const setOffset = jest.fn()

const setup = () => {
const props = {
limit: 2,
count: 14,
setoffset
setOffset
}
render(
<PaginationComponent {...props} />
Expand Down
50 changes: 29 additions & 21 deletions static/src/js/components/Table/Table.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,39 @@ import For from '../For/For'
/**
* Table
* @typedef {Object} Table
* @property {Array} headers A list of custom headers
* @property {Array} classNames A list of classNames for each header
* @property {Boolean} loading A value provided by whichever useQuery is being used in parent component
* @property {Object[]} headers A list of header titles for a table
* @property {Array} data An array of formatted rows with data already populated
* @property {String} error A string with a specific error for table
* @property {String} error A string that comes from { loading, data, error } = useQuery
* @property {String} noDataError An option string for custom error if data.length === 0
* @property {Number} count A number of the total amount of data without limit
* @property {Function} setOffset A useState function that loads the appropriate data set based on user input
* @property {limit} limit A number that limits the data set that comes in. **Note this number should always be 20
* @property {limit} offset A number that dictates where the dataset should start from
*/

const Table = ({
headers, classNames, loading, data, error, count, setOffset, limit, offset
headers,
classNames,
loading,
data,
error,
noDataError,
count,
setOffset,
limit,
offset
}) => {
// Current page of rows
const pagedRows = data

// Does this provider have enough Rows to page? We hide the pagination buttons if not
const hasPages = count > limit

const renderHeaders = headers.map((header) => (
<th key={header}>{header}</th>
))

const dataLengthExists = (data && data.length) && data.length

const content = []

if (loading) {
Expand Down Expand Up @@ -60,8 +74,8 @@ const Table = ({
}
</For>
)
} else if (pagedRows.length > 0) {
const rowData = pagedRows.map((row) => {
} else if (dataLengthExists > 0) {
const rowData = data.map((row) => {
const { cells, key } = row
const rowKey = key

Expand All @@ -84,6 +98,8 @@ const Table = ({
})

content.push(rowData)
} else if (dataLengthExists === 0) {
content.push(<tr key="error-banner" className="text-center"><td colSpan={4}>{noDataError}</td></tr>)
} else {
content.push(<tr key="error-banner" className="text-center"><td colSpan={4}>{error}</td></tr>)
}
Expand All @@ -93,25 +109,15 @@ const Table = ({
<Row>
<Col>
<span className="d-block mb-3">
Showing
{' '}
{count > 0 ? offset : 0}
-
{data.length === limit ? offset + limit : offset + data.length}
{' '}
of
{' '}
{count}
{' '}
Results
{`Showing ${count > 0 ? offset : 0}-${dataLengthExists === limit ? offset + limit : offset + dataLengthExists} of ${count} Results`}
</span>
</Col>
<Col xs="auto">
<div className="mx-auto">
{
hasPages && (
<PaginationComponent
setoffset={setOffset}
setOffset={setOffset}
limit={limit}
count={count}
/>
Expand Down Expand Up @@ -141,7 +147,8 @@ const Table = ({
Table.defaultProps = {
classNames: [],
loading: null,
error: 'No Data to Display',
error: 'Error',
noDataError: 'No Data to Display',
count: null
}

Expand All @@ -154,6 +161,7 @@ Table.propTypes = {
cells: PropTypes.arrayOf(PropTypes.shape({}))
})).isRequired,
error: PropTypes.string,
noDataError: PropTypes.string,
count: PropTypes.number,
offset: PropTypes.number.isRequired,
setOffset: PropTypes.func.isRequired,
Expand Down
14 changes: 13 additions & 1 deletion static/src/js/components/Table/__tests__/Table.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('Table', () => {
expect(screen.getByText('column 1')).toBeInTheDocument()
expect(screen.getByRole('table')).toBeInTheDocument()
expect(screen.getByText('Showing 0-2 of 14 Results')).toBeInTheDocument()
expect(screen.getByText('Row 2 Cell 2')).toBeInTheDocument()
})
})

Expand All @@ -67,13 +68,24 @@ describe('Table', () => {
setup({
data: [],
count: 0,
error: 'Custom Error Message'
noDataError: 'Custom Error Message'
})

expect(screen.getByText('Custom Error Message')).toBeInTheDocument()
})
})

describe('when the table component has an error from useQuery', () => {
test('renders error message', () => {
setup({
data: null,
error: 'Error Message'
})

expect(screen.getByText('Error Message')).toBeInTheDocument()
})
})

describe('when the table component is passed loading:true', () => {
test('renders loading screen', () => {
setup({
Expand Down

0 comments on commit 868cbae

Please sign in to comment.