POC: score GradedGroup externally using callback #1981
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
handeyeco
changed the title
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (dff336d) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1981If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1981 |
|
Size Change: +111 B (+0.01%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
jeremywiebe
approved these changes
Dec 11, 2024
| @@ -28,6 +28,7 @@ export type DependencyProps = Partial< | |||
|
|
|||
| export const DependenciesContext = React.createContext<PerseusDependenciesV2>({ | |||
| analytics: {onAnalyticsEvent: async () => {}}, | |||
| gradingCallback: () => ({type: "invalid"}), | |||
There was a problem hiding this comment.
Suggested change
| gradingCallback: () => ({type: "invalid"}), | |
| scoringCallback: () => ({type: "invalid"}), |
|
|
||
| const userInput = this.rendererRef.current?.getUserInputMap(); | ||
| const score: PerseusScore = this.props.gradingCallback( | ||
| this.props.widgetId, |
There was a problem hiding this comment.
🤔 This means that the handler of this callback will need to go through the article/item to find the widget config, plus it will need to deal with upgrading props before scoring.
What do you think of passing this.props instead of the widget ID so that the handler has everything it needs in the callback to call the scoring function?
There was a problem hiding this comment.
In thinking about this a bit more, I'm not sure that'll work. The props may not have the data to score. I do think it'll be slightly more complicated than just an id though. These widgets can show up inside hints, or the graded-group-set or articles. We'll need a way to uniquely identify where this widget lives so that the chrome can correctly identify which scoring data to use.
## Summary: Remove `guessAndScore` from Renderer. This hasn't been used in learner-facing logic for a little bit now. The bulk of this PR is updating internal usage - tools for testing exercise correctness were using this (specifically in Cypress and our custom matchers for Jest); so most of the work was porting tests to use `scorePerseusItem` instead. ## Test plan: From a learner's perspective, this is dead code. So while this is a major API change, it's mostly removing dead code and updating tests. Tests should continue to run successfully. Author: handeyeco Reviewers: handeyeco, Myranae, jeremywiebe Required Reviewers: Approved By: Myranae, jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #1974
handeyeco
changed the base branch from
rm-renderer-scoring
to
remove-scoreinput2
An error occurred while trying to automatically change base from
rm-renderer-scoring
to
remove-scoreinput2
December 11, 2024 20:55
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This adds a new dependency for scoring that GradedGroup can call when wanting to check answers in an Article
Issue: LEMS-2719
Test plan: