Avoid creating multiple forms #2883
Conversation
victoria34
force-pushed
the
google-form
branch
from
d7142eb
to
cdc316b
Compare
| @@ -48,6 +53,10 @@ export default class TrialMatchTable extends React.Component<ITrialMatchProps> { | |||
| } | |||
|
|
|||
| @observable showTrialFeedback = false; | |||
| @observable selectedTrial: ISelectedTrial = { | |||
There was a problem hiding this comment.
by default this should be undefined
| @@ -196,6 +196,12 @@ export default class TrialMatchTable extends React.Component<ITrialMatchProps> { | |||
| ); | |||
| } | |||
|
|
|||
| public openFeedbackForm(nctId: string, protocolNo: string) { | |||
| this.showTrialFeedback = true; | |||
There was a problem hiding this comment.
instead of having a flag, you can just can just have selectedTrialFeedbackFormData which is either undefined (hide) or shown (populated). In general, it's best not to update multiple properties like this.
There was a problem hiding this comment.
Removed the flag
| @@ -196,6 +196,12 @@ export default class TrialMatchTable extends React.Component<ITrialMatchProps> { | |||
| ); | |||
| } | |||
|
|
|||
| public openFeedbackForm(nctId: string, protocolNo: string) { | |||
There was a problem hiding this comment.
this modifies mobx state and so should have @action decorator
victoria34
force-pushed
the
google-form
branch
from
efa26f3
to
7c5a0af
Compare
victoria34
force-pushed
the
google-form
branch
from
7c5a0af
to
d0cd18b
Compare
| @@ -335,6 +339,17 @@ export default class TrialMatchTable extends React.Component<ITrialMatchProps> { | |||
| title="OncoKB Matched Trials General Feedback" | |||
| userEmailAddress={AppConfig.serverConfig.user_email_address} | |||
| /> | |||
| { this.selectedTrialFeedbackFormData && | |||
| <TrialMatchFeedback | |||
| show={!!this.selectedTrialFeedbackFormData} | |||
There was a problem hiding this comment.
don't need show prop and internal logic anymore.
There was a problem hiding this comment.
I added the logic here because the window always popup the Exit Confirmation modal.
| { this.selectedTrialFeedbackFormData && | ||
| <TrialMatchFeedback | ||
| show={!!this.selectedTrialFeedbackFormData} | ||
| onHide={() => !!(this.selectedTrialFeedbackFormData=undefined)} |
There was a problem hiding this comment.
onHide also modifies state so should be action. What I would do is change openFeedbackForm to openCloseFeedbackForm and have it accept undefined if it's close
victoria34
force-pushed
the
google-form
branch
2 times, most recently
from
c05d1e1
to
aa786e3
Compare
victoria34
force-pushed
the
google-form
branch
from
aa786e3
to
cea7ec0
Compare
Avoid creating multiple popup modals