Remove unused rubric type for iFrame #1996
Conversation
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (70571d9) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1996If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1996 |
| @@ -102,9 +102,6 @@ export type PerseusGrapherRubric = { | |||
|
|
|||
| export type PerseusGrapherUserInput = PerseusGrapherRubric["correct"]; | |||
|
|
|||
| // TODO(LEMS-2440): Can possibly be removed during 2440; only userInput used | |||
| export type PerseusIFrameRubric = Empty; | |||
There was a problem hiding this comment.
Could removing this now cause problems down the road? I think it's pretty easy to add it back in, but not sure if we need a rubric and userInput for every widget. I'm guessing it's okay, but wanted to double check.
There was a problem hiding this comment.
I agree to remove it now. We already have other widgets where there is no Rubric type.
|
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@khanacademy/perseus": patch | |||
There was a problem hiding this comment.
This seems to just be removing an unused type, so I put it as patch, but it does touch the UserInput type and the Props type, so wasn't sure if that meant it should be minor. Leaning towards patch, but wanted to confirm :)
There was a problem hiding this comment.
Would it be useful to change what the scoreIframe parameter is called as it is not actually the user's input, but a summary of the results of checking the user's input?
No, please don't change the scoring function parameter names. They should all conform to the WidgetScorerFunction type:
export type WidgetScorerFunction = (
// The user data needed to score
userInput: UserInput,
// The scoring criteria to score against
rubric: Rubric,
// Strings, for error messages in invalid widgets
string?: PerseusStrings,
// Locale, for math evaluation
// (1,000.00 === 1.000,00 in some countries)
locale?: string,
) => PerseusScore;We can omit parameters from the end of the list (all the up to having a scoring function that takes no parameters). Also, TypeScript doesn't care about the parameter names, but for consistency let's keep the argument list and argument names the same as in the WidgetScorerFunction. :)
| "@khanacademy/perseus": patch | ||
| --- | ||
|
|
||
| Remove unused iframe rubric type |
There was a problem hiding this comment.
I don't see any changes to our main package file (index.ts) which means that this type was not exported from Perseus, so a patch release sounds right to me.
…arate-answers-from-user-input
…arate-answers-from-user-input
Summary:
The initial goal was to separate out answers from the userInput, but iFrame is a unique widget in that the answers aren't actually contained in what is sent to the scoring function. Scoring happens via the iFrame, and the scoring function is told if the user is correct, incorrect, and incomplete. Additionally, validation happens after both types of scoring, so it most likely will not be possible to have a validation function for this widget either.
Question:
Would it be useful to change what the
scoreIframeparameter is called as it is not actually the user's input, but a summary of the results of checking the user's input? We could revert the argument back tostateor mayberesults, but then would we change the type name? I wonder if that would be confusing down the road, but we do have comments explaining it somewhat.Issue: LEMS-2440
Test plan: