-
Notifications
You must be signed in to change notification settings - Fork 51
fix(PM-2353): reset filter on viewing list view #1688
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
base: v6
Are you sure you want to change the base?
Conversation
} | ||
|
||
openChallengeView (challenge) { | ||
this.props.setActiveProject(parseInt(challenge.projectId)) |
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.
The openChallengeView
method should be bound to this
in the constructor if it is intended to be used as an event handler or passed around as a callback. This ensures the correct this
context is maintained.
} | ||
|
||
openChallengeView (challenge) { | ||
this.props.setActiveProject(parseInt(challenge.projectId)) |
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.
Consider adding error handling for the parseInt(challenge.projectId)
call to ensure that challenge.projectId
is a valid number and handle cases where it might not be.
</div> | ||
|
||
<Link className={styles.col2} to={`/projects/${challenge.projectId}/challenges/${challenge.id}/view`} onClick={() => setActiveProject(parseInt(challenge.projectId))}> | ||
<Link className={styles.col2} to={`/projects/${challenge.projectId}/challenges/${challenge.id}/view`} onClick={() => this.openChallengeView(challenge)}> |
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.
The function openChallengeView
is being called with challenge
as an argument. Ensure that openChallengeView
is defined and handles the challenge
parameter correctly. If openChallengeView
is not defined within this component, it should be imported or defined appropriately.
!isReadOnly && ( | ||
<div className={styles.col6}> | ||
{(disableHover ? <Link className={styles.link} to={`/projects/${challenge.projectId}/challenges/${challenge.id}/edit`}>Edit</Link> : hoverComponents(challenge, this.onUpdateLaunch, this.deleteModalLaunch))} | ||
{(disableHover ? <Link className={styles.link} to={`/projects/${challenge.projectId}/challenges/${challenge.id}/edit`} onClick={() => this.props.resetFilter()}>Edit</Link> : hoverComponents(challenge, this.onUpdateLaunch, this.deleteModalLaunch))} |
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.
Consider checking if this.props.resetFilter
is defined before calling it to prevent potential runtime errors if the prop is not passed correctly. For example: onClick={() => this.props.resetFilter && this.props.resetFilter()}
.
auth: PropTypes.object.isRequired, | ||
loginUserRoleInProject: PropTypes.string | ||
loginUserRoleInProject: PropTypes.string, | ||
resetFilter: PropTypes.func |
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.
The resetFilter
prop type should be more specific. Consider defining the expected shape or type for this function using PropTypes.func.isRequired
if it is mandatory, or PropTypes.func
if it is optional.
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-2353