feat: include badge showing applied filters in Verify page #173
Conversation
tranquanghuy0801
requested review from
gwynndp,
luacmartins and
nmcharlton
as code owners
src/common/utils.js
Outdated
| } | ||
|
|
||
| Object.keys(filter).forEach(key => { | ||
| if (key === 'filter' || key == 'active' || key == 'approved') { |
There was a problem hiding this comment.
It's best to always use === for string comparisons. Our linter should pick up on this.
src/common/utils.js
Outdated
| @@ -8,4 +8,36 @@ export const getVerificationStatus = (active, approved) => { | |||
| } else if (active === false && approved === false) { | |||
| return verificationStates.REJECTED; | |||
| } | |||
| }; | |||
|
|
|||
| export const countAppliedFilters = (filter) => { | |||
There was a problem hiding this comment.
I would suggest that this be a function of the Filter model to keep it all in one place.
In its current form, this could easily be broken if a developer makes changes to Filter and isn't aware of this function.
There was a problem hiding this comment.
It would also be good to create a test for this function.
src/components/Verify.js
Outdated
| countAppliedFilters(verifyContext.filter) > 0 ? | ||
| <Avatar className={classes.activeFilters}> | ||
| { | ||
| countAppliedFilters(verifyContext.filter) |
There was a problem hiding this comment.
There's no need to call the function twice.
Assign the result to a variable and check that instead.
src/components/Verify.js
Outdated
| color="primary" | ||
| onClick={handleFilterClick} | ||
| startIcon={<IconFilter />} | ||
| key={1} | ||
| > | ||
| Filter | ||
| { | ||
| countAppliedFilters(verifyContext.filter) > 0 ? |
There was a problem hiding this comment.
This can be expressed as:
(countAppliedFilters(verifyContext.filter) > 0) && <Avatar...
There's no need for the ternary operator.
src/common/utils.js
Outdated
| numFilters += 1; | ||
| } else if ((key === 'dateStart' || key === 'dateEnd') && filter[key] !== undefined) { | ||
| numFilters += 1; | ||
| } else if (key == 'organizationId' && filter[key] !== 'ALL_ORGANIZATIONS') { |
There was a problem hiding this comment.
We have ALL_ORGANIZATIONS and ALL_SPECIES defined as variables in src/models/filter.js. It's best to use those in place of these string literals.
Resolves #81