-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reset filters button #82
Conversation
# Conflicts: # src/pages/MyTransfers/TableFilters.js
return ( | ||
<FilterResetButton | ||
onClick={() => setFilter(defaultFilter)} | ||
type='submit' |
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.
Maybe it should be 'button' instead as there is no form data to submit
|
||
export const FilterResetButton = styled(Button)({ | ||
color: '#fff', | ||
height: '40px', |
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 see that you mostly use px in a markup, and it is maybe not a problem here as it is a small button, but I would suggest using relative units instead like rem, so it would look similar on different scales.
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 believe the reset button is typically found on the right-hand side of the filters. Anyway, the design of the filters will most likely be changed in V2, but if it is not a problem, I would move this button to the right.
Description
A button to reset My Transfer table filters is implemented.
Issue(s) addressed
What kind of change(s) does this PR introduce?
Please check if the PR fulfills these requirements
Issue
What is the current behavior?
Currently, there is no way to reset the table filters without manually changing the values.
What is the new behavior?
Breaking change
Does this PR introduce a breaking change?
No.
Other useful information
None.