Add an option to select a timeline template #1072
Conversation
There was a problem hiding this comment.
-
At the moment only default timeline templates are loaded as per Default Timeline Templates #881. As a result inside timeline selects we can see only one default option and cannot change the timeline template.
-
If I change the URL for loading timeline templates https://github.com/topcoder-platform/work-manager/blob/feature/timeline-template/src/services/challenges.js#L94 (remove
isDefault=true). Then selects would show more timelines to select. This would reveal some issue:- choose "Design" and "Challenge" - see the correct templates for Design
- change track to "Data Science" - see correct templates for DS
- change track again to "Design" - templates are still showing for DS, not for Design
See demo video https://monosnap.com/file/4DqtnNo7PUKk5lxweMbzfregakW8z
| shouldComponentUpdate (nextProps) { | ||
| const { onUpdateSelect, challengeTimelines, timelineTemplates, challenge } = nextProps | ||
| const hasSelectedTypeAndTrack = !_.isEmpty(challenge.typeId) && !_.isEmpty(challenge.trackId) | ||
| if ((hasSelectedTypeAndTrack && this.state.validOptions.length === 0) || this.state.matchString !== `${challenge.typeId}-${challenge.trackId}-${this.state.selectedOption.value}`) { | ||
| this.checkData(onUpdateSelect, challengeTimelines, timelineTemplates, challenge) | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
It seems that shouldComponentUpdate is used as a replacement for unsafecomponentWillUpdate and it triggers this.setState and external callback onUpdateSelect inside. As per my understanding such approach is not recommended and in particular it could lead to a circular re-reredering.
Here is some guide on which approaches are recommended as a replacement for unsafe methods https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.
It includes examples for updating state based on props and invoking external callbacks.
… challenge, task, and first2finish due to this
|
Merging to push the changes to prod as separate release now. |
No description provided.