Fix/update kd quiz question#213
Conversation
meganrm
requested review from
ShrimpCryptid,
Copilot and
tyler-foster
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
Updates the competitive binding (Module.A_B_D_AB) learning flow to ask for Kᵢ instead of Kd, and enforces collecting a baseline (0 inhibitor) measurement before allowing the inhibitor concentration to be adjusted.
Changes:
- Added a new KiQuestion component and conditionally render it for the competitive binding module.
- Disabled the inhibitor concentration slider in module 3 until a baseline (0) concentration has been recorded, with a hover help popup explaining why.
- Renamed the gating prop/variable from
canDetermineEquilibrium/canDetermineKdto a more generalcanDetermineConstant.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/quiz-questions/KiQuestion.tsx | New quiz question component for Kᵢ in the competitive binding module. |
| src/components/main-layout/CenterPanel.tsx | Switches between Kd/Ki question based on the active module. |
| src/components/concentration-display/Concentration.tsx | Disables the slider in module 3 until baseline is recorded. |
| src/components/concentration-display/ConcentrationSlider.tsx | Adds disabled support and a help popup explaining baseline requirement. |
| src/components/HelpPopup.tsx | Updates HelpPopup API to use open/trigger. |
| src/components/main-layout/RightPanel.tsx | Adapts HelpPopup usage to the updated open prop. |
| src/App.tsx | Renames determination gating logic to canDetermineConstant and updates downstream usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleAnswerSelection = (answer: valueType | null) => { | ||
| setSelectedAnswer(Number(answer)); | ||
| if (formState === FormState.Incorrect) { | ||
| setFormState(FormState.Clear); | ||
| } | ||
| }; |
| <InputNumber | ||
| aria-labelledby="ki question" | ||
| value={selectedAnswer || ""} | ||
| onChange={handleAnswerSelection} | ||
| placeholder="Type value..." | ||
| /> |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ShrimpCryptid
left a comment
ShrimpCryptid
left a comment
There was a problem hiding this comment.
Left some non-blocking comments!
| } else { | ||
| setFormState(FormState.Incorrect); | ||
| } |
There was a problem hiding this comment.
So just to check, if the student submits a wrong answer, it will show an incorrect state, and if the student hits submit again, it will clear the state?
Does QuizForm change the button message text when in the incorrect state?
There was a problem hiding this comment.
yes the button says "try again"
| successMessage={getSuccessMessage(selectedAnswer!)} | ||
| failureMessage='Visit the "Learn how to derive Ki" button above, then use the Equilibrium concentration plot to answer.' | ||
| formState={formState} | ||
| id="Ki Value" |
There was a problem hiding this comment.
Can id strings contain spaces? Or is this being used differently by QuizForm?
There was a problem hiding this comment.
Ah, looked in QuizForm. Out of scope of this PR but I feel like id has two uses that are both different from the HTML id convention. Maybe it would be better to split it into id and minimizedTitle props?
There was a problem hiding this comment.
yeah, that seems like a good idea
Problem
Estimated review size: md
closes #205
Solution
Type of change
Please delete options that are not relevant.
Steps to Verify:
Screenshots (optional):