Skip to content
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

Move Radio scorer/validator #2106

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Conversation

handeyeco
Copy link
Contributor

@handeyeco handeyeco commented Jan 14, 2025

Summary:

Move Radio scorer/validator to perseus-score

Issue: LEMS-2737

Test plan:

Nothing should change, just moving code

@handeyeco handeyeco self-assigned this Jan 14, 2025
@handeyeco handeyeco changed the title move radio scorer/validator Move Radio scorer/validator Jan 14, 2025
@handeyeco handeyeco requested review from a team January 14, 2025 22:17
Copy link
Contributor

github-actions bot commented Jan 14, 2025

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (96fc81f) and published it to npm. You
can install it using the tag PR2106.

Example:

yarn add @khanacademy/perseus@PR2106

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR2106

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Size Change: +43 B (0%)

Total Size: 1.47 MB

Filename Size Change
packages/kmath/dist/es/index.js 86.8 kB +3.71 kB (+4.47%)
packages/math-input/dist/es/index.js 77.6 kB -355 B (-0.46%)
packages/perseus-core/dist/es/index.js 27.1 kB +4.03 kB (+17.47%) ⚠️
packages/perseus-editor/dist/es/index.js 689 kB -152 B (-0.02%)
packages/perseus-score/dist/es/index.js 113 kB +6.46 kB (+6.08%) 🔍
packages/perseus/dist/es/index.js 394 kB -13.7 kB (-3.36%)
packages/perseus/dist/es/strings.js 4.87 kB +39 B (+0.81%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 39 kB
packages/keypad-context/dist/es/index.js 760 B
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-linter/dist/es/index.js 22.2 kB
packages/pure-markdown/dist/es/index.js 3.66 kB
packages/simple-markdown/dist/es/index.js 12.5 kB

compressed-size-action

Copy link
Member

@benchristel benchristel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

};

const errorToString: ErrorStringMap = {
MISSING_PERCENT_ERROR: strings.MISSING_PERCENT_ERROR as string,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of being forced to as string each of these, I think we can fix this more cleanly by moving the type annotation for strings from:

export const strings: {
    [key in keyof PerseusStrings]:
        | string
        | {context?: string; message: string}
        | {context?: string; one: string; other: string};
} = {
    closeKeypad: "close math keypad",
    ...
};

to:

export const strings = {
    closeKeypad: "close math keypad",
    ...
} satisfies {
    [key in keyof PerseusStrings]:
        | string
        | {context?: string; message: string}
        | {context?: string; one: string; other: string};
};

Using satisfies instead of : mytype = means that we allow TypeScript to infer the type of each key in strings, but the entire structure must conform to the satisfied type. So TypeScript can now infer that strings has a closeKeypad key with a string value instead of a value that is one of string {context..} etc.

* cs-program

* move dropdown

* move iframe

* Move scoring logic: Table, NumberLine, Matcher (#2112)

* move table

* number-line

* move matcher

* Move scorers: Plotter and Sorter (#2113)

* plotter

* sorter

* Move scorer: Orderer (#2114)

* orderer

* Move scorerer: LabelImage (#2115)

* move label-image

* rename labelImageScoreMarker

* Move scoring logic: Matrix (#2116)

* move matrix

* Move scoring logic: Expression (#2118)

* move expression scorer

* respond to Ben's feedback

* merge Grapher move

* fix conflict again

* Move scorer: Grapher (#2119)

* move matrix

* move expression scorer

* move grapher scorer

* respond to Ben's feedback

* Move scorer: Interactive Graph (#2120)

* STOPSHIP some type errors still

* add back duplicate declarations

* add back Line duplicate

* all tests passing

* Revert changes to underscore's isEqual (#2125)

## Summary:
[Original comment](#2113 (comment))

I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once.

Issue: LEMS-2737

## Test plan:

Author: handeyeco

Reviewers: jeremywiebe, benchristel

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (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] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2125

* respond to Jeremy's feedback
@handeyeco handeyeco merged commit 8049d09 into LEMS-2737/numeric-input Jan 17, 2025
4 of 6 checks passed
@handeyeco handeyeco deleted the LEMS-2737/radio branch January 17, 2025 21:21
handeyeco added a commit that referenced this pull request Jan 17, 2025
…2105)

* move numeric-input and input-number

* respond to Jeremy's feedback

* Move Radio scorer/validator (#2106)

* move radio scorer/validator

* lint errorToString better

* comment my unique type

* respond to Jeremy's feedback

* Move scoring logic: CSProgram, Iframe, and Dropdown (#2111)

* cs-program

* move dropdown

* move iframe

* Move scoring logic: Table, NumberLine, Matcher (#2112)

* move table

* number-line

* move matcher

* Move scorers: Plotter and Sorter (#2113)

* plotter

* sorter

* Move scorer: Orderer (#2114)

* orderer

* Move scorerer: LabelImage (#2115)

* move label-image

* rename labelImageScoreMarker

* Move scoring logic: Matrix (#2116)

* move matrix

* Move scoring logic: Expression (#2118)

* move expression scorer

* respond to Ben's feedback

* merge Grapher move

* fix conflict again

* Move scorer: Grapher (#2119)

* move matrix

* move expression scorer

* move grapher scorer

* respond to Ben's feedback

* Move scorer: Interactive Graph (#2120)

* STOPSHIP some type errors still

* add back duplicate declarations

* add back Line duplicate

* all tests passing

* Revert changes to underscore's isEqual (#2125)

## Summary:
[Original comment](#2113 (comment))

I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once.

Issue: LEMS-2737

## Test plan:

Author: handeyeco

Reviewers: jeremywiebe, benchristel

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (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] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2125

* respond to Jeremy's feedback
handeyeco added a commit that referenced this pull request Jan 17, 2025
* move categorizer scoring logic

* Move NumericInput and InputNumber scoring logic to `perseus-score` (#2105)

* move numeric-input and input-number

* respond to Jeremy's feedback

* Move Radio scorer/validator (#2106)

* move radio scorer/validator

* lint errorToString better

* comment my unique type

* respond to Jeremy's feedback

* Move scoring logic: CSProgram, Iframe, and Dropdown (#2111)

* cs-program

* move dropdown

* move iframe

* Move scoring logic: Table, NumberLine, Matcher (#2112)

* move table

* number-line

* move matcher

* Move scorers: Plotter and Sorter (#2113)

* plotter

* sorter

* Move scorer: Orderer (#2114)

* orderer

* Move scorerer: LabelImage (#2115)

* move label-image

* rename labelImageScoreMarker

* Move scoring logic: Matrix (#2116)

* move matrix

* Move scoring logic: Expression (#2118)

* move expression scorer

* respond to Ben's feedback

* merge Grapher move

* fix conflict again

* Move scorer: Grapher (#2119)

* move matrix

* move expression scorer

* move grapher scorer

* respond to Ben's feedback

* Move scorer: Interactive Graph (#2120)

* STOPSHIP some type errors still

* add back duplicate declarations

* add back Line duplicate

* all tests passing

* Revert changes to underscore's isEqual (#2125)

## Summary:
[Original comment](#2113 (comment))

I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once.

Issue: LEMS-2737

## Test plan:

Author: handeyeco

Reviewers: jeremywiebe, benchristel

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (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] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2125

* respond to Jeremy's feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants