-
Notifications
You must be signed in to change notification settings - Fork 16
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
HELP-Med show questions #295
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
@@ -330,7 +330,9 @@ const ChatWithInstructions: FC< | |||
<div className="flex items-end justify-end gap-4"> | |||
<GeneralButton | |||
text="I understand" | |||
onClick={() => setReadInstructions(true)} | |||
onClick={() => {setReadInstructions(true); | |||
setIsGenerativeContext(false);} |
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.
If an user hasn't click on save and then finish, and clicks on the submit button at the end of the page the interaction with the bot wouldn't be properly saved. Please remove line 334
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.
Please review comments
Good point. The other changes aren't useful without line 334, so I'll just cancel this PR. It would be nice to be able to show the questions right away because some users keep trying to put their answers in the chatbots, but changing the "Submit" button seems like a bad idea right now, so I'll keep it as is. |
Making the questions appear as soon as the instructions have been read. This uses a Boolean that seems to be misnamed. For some reason, whether or not the questions appear is controlled by
isGenerativeContext
. To avoid breaking anything else, I have kept the name as it is, but this seems like something that should be refactored eventually.