-
Notifications
You must be signed in to change notification settings - Fork 2
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
Filtering #59
Filtering #59
Conversation
# Conflicts: # actionlist/app/components/SideMenu.js
# Conflicts: # actionlist/app/components/SideMenu.js # actionlist/package.json
# Conflicts: # actionlist/app/actions/action_items.js # actionlist/app/actions/types.js # actionlist/app/components/SideMenu.js # actionlist/app/reducers/action_items.js # actionlist/package.json
onSort: criteria => dispatch(sortActionList(criteria)), | ||
}); | ||
|
||
/* eslint-enable no-unused-vars */ |
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.
Remove this eslint exception
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.
fixed!!
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.
This PR looks good. If you would, please, just extract the repeated<View style={style.header}><Text style={style.headerText}>{/* text */}</Text></View>
segments into a Header
component so they can be reused consistently.
|
||
const ContentHeader = ({ title }) => ( | ||
<View> | ||
<Text style={style.headerText}>{title}</Text> |
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'd recommend not using a title
prop and just use the children
prop. It looks like it's allowed for Text
components to have node children, so if you just use children you can pass off prop validation to the react-native component and allow yourself more flexibility.
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 tried to use children and lint threw an error saying I need to have a prop validation for it
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.
Yes, the type of children
should be documented as React.PropTypes.node
: jsx-eslint/eslint-plugin-react#7
# Conflicts: # actionlist/app/containers/PreferencesContainer.js
3 filters implemented. The ones based on list options (documentRouteStatus, documentType and actionRequested)