add default terms and refractor nda input #1054
Conversation
There was a problem hiding this comment.
@CDharmateja there are some missing requrements, let me clarify them:
-
We should not show all the terms and should be allowed to edit all the terms. Only enable/disable NDA. So no need to show terms and we should hide the editor for all the terms.
- As we don't need to show Terms selector, please, cancel all the changes in the file
src/components/ChallengeEditor/Terms-Field/index.jsfor now.
- As we don't need to show Terms selector, please, cancel all the changes in the file
-
There is one missing requirement:
When we are creating a new challenge, we should apply all the terms of the projects which are listed in the project.terms array to the challenge. So new challenge has to be created with the same terms as project.
So when we create a challenge now, we add only one hardcoded term https://github.com/topcoder-platform/work-manager/blob/develop/src/components/ChallengeEditor/index.js#L838. We should still add this term. But also, we should add all the terms of project, which are stored in
project.terms. Each term should be added with the same roleIdSUBMITTER_ROLE_UUID. -
In the VIEW mode, let's show only text "yes" or "no" without radioboxes, like we do for other data
-
Can we please make smaller distance between yes and no options
| {params.get('beta') === 'true' && ( | ||
| <div className={styles.row}> | ||
| <div className={styles.col}> | ||
| <span> | ||
| <span className={styles.fieldTitle}>Terms:</span> | ||
| {challenge.terms.map(term => term.id).join(', ')} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
no need to show all the terms
| {params.get('beta') && ( | ||
| <TermsField | ||
| terms={metadata.challengeTerms} | ||
| projectTerms={projectDetail.terms} | ||
| challenge={challenge} | ||
| onUpdateMultiSelect={this.onUpdateMultiSelect} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
no need to edit al the terms, please, hide it using flase as it was before
|
Also, could you please, resolve merge conflicts with the other PR which I've already merged. |
There was a problem hiding this comment.
Thanks for update, @CDharmateja. Works great now. Just a few minor improvements:
-
Label for this field should be "bold" as all other labels:
Also, there are few code improvement suggestions, please, kindly check comments below.
| ? <div>Yes</div> | ||
| : <div>No</div> |
There was a problem hiding this comment.
@CDharmateja I guess we don't need to wrap each answer "yes" / "no" into <div> and can simplify this to:
{isRequiredNda ? 'Yes' : 'No'}| /> | ||
| <label className={styles['tc-RadioButton-label']} htmlFor='nda-yes'> | ||
| <div>yes</div> | ||
| <input type='hidden' /> |
There was a problem hiding this comment.
@CDharmateja do we really need this "hidden" input?
There was a problem hiding this comment.
I think its not necessary. The input radio structure is like that here
. So I have used it.| /> | ||
| <label className={styles['tc-RadioButton-label']} htmlFor='nda-no'> | ||
| <div>No</div> | ||
| <input type='hidden' /> |
There was a problem hiding this comment.
@CDharmateja do we really need this "hidden" input?
#1044