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

[LEMS-2563] Primary widget splitter #2234

Merged
merged 8 commits into from
Feb 14, 2025
Merged

[LEMS-2563] Primary widget splitter #2234

merged 8 commits into from
Feb 14, 2025

Conversation

handeyeco
Copy link
Contributor

@handeyeco handeyeco commented Feb 12, 2025

Summary:

Create a central place to pass in a PerseusItem and return a version with all the widget options split of answer data.

Mostly tests and some moving of logic. The main important thing is splitPerseusItem.

Issue: LEMS-2563

Test plan:

This just adds the logic, still not using it. Just unit tests right now.

@handeyeco handeyeco self-assigned this Feb 12, 2025
Copy link
Contributor

github-actions bot commented Feb 12, 2025

Size Change: +29 B (0%)

Total Size: 1.47 MB

Filename Size Change
packages/perseus-core/dist/es/index.js 46.8 kB +399 B (+0.86%)
packages/perseus/dist/es/index.js 368 kB -370 B (-0.1%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 39 kB
packages/keypad-context/dist/es/index.js 760 B
packages/kmath/dist/es/index.js 86.9 kB
packages/math-input/dist/es/index.js 77.7 kB
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-editor/dist/es/index.js 689 kB
packages/perseus-linter/dist/es/index.js 22.2 kB
packages/perseus-score/dist/es/index.js 115 kB
packages/perseus/dist/es/strings.js 6.57 kB
packages/pure-markdown/dist/es/index.js 3.66 kB
packages/simple-markdown/dist/es/index.js 12.5 kB

compressed-size-action

@handeyeco handeyeco changed the title [LEMS-2563/main-splitter] attach splitters to widgetLogic [LEMS-2563] Primary widget splitter Feb 12, 2025
@handeyeco handeyeco requested review from jeremywiebe, benchristel, Myranae and a team February 13, 2025 16:41
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.

Looks good! I left a few non-blocking suggestions inline.

packages/perseus-core/src/utils/split-perseus-item.ts Outdated Show resolved Hide resolved
packages/perseus-core/src/utils/split-perseus-item.ts Outdated Show resolved Hide resolved
Comment on lines 36 to 38
item.widgets = splitWidgets;

return item;
Copy link
Member

Choose a reason for hiding this comment

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

If we did this:

Suggested change
item.widgets = splitWidgets;
return item;
return {...item, widgets: splitWidgets};

we wouldn't need to clone above. It might also be clearer what parts of item are being changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kind of did a middle ground on this, returning an object literal to make it easier to read but I'm still cloning the object because I'm paranoid.


import type {PerseusRenderer} from "../data-schema";

export default function splitPerseusItem(
Copy link
Member

Choose a reason for hiding this comment

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

Should this function be called splitPerseusRenderer? Or perhaps getRendererPublicData for consistency with the get*PublicWidgetOptions functions?

Suggested change
export default function splitPerseusItem(
export default function getRendererPublicData(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did splitPerseusItem to be consistent with scorePerseusItem and validatePerseusItem. If it's all the same to you, I think I'll keep it as-is. As an aside, I think both Renderer and PublicData are misnomers.

  • PerseusRenderer isn't a renderer, it's an object that describes an article/exercise that's passed to a renderer. Most of Webapp calls it item or itemData which is a little better IMO but also a little more vague.
  • Both the full data and PublicOptions are public data, they're just public at different times. I think StrippedData or AnswerlessData would be more appropriate, but they both have their own issues.

Copy link
Contributor

github-actions bot commented Feb 13, 2025

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR2234

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

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

@handeyeco handeyeco merged commit 1ade12c into main Feb 14, 2025
8 checks passed
@handeyeco handeyeco deleted the LEMS-2563/main-splitter branch February 14, 2025 19:18
catandthemachines added a commit that referenced this pull request Feb 14, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @khanacademy/[email protected]

### Major Changes

- [#2234](#2234)
[`1ade12c18`](1ade12c)
Thanks [@handeyeco](https://github.com/handeyeco)! - Move splitters into
perseus-core, add splitPerseusItem

### Minor Changes

- [#2228](#2228)
[`edd34241e`](edd3424)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Remove legacy graph


- [#2238](#2238)
[`8e4cb7f53`](8e4cb7f)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Bugfixes to ensure that focus is handled correctly on unlimited
point/polygon graphs.

### Patch Changes

- [#2223](#2223)
[`f8a4becb0`](f8a4bec)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! -
[Bugfix] Interactive Graph crashes in editor when setting domain for
locked function


- [#2201](#2201)
[`91cede41f`](91cede4)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Refactor - Update
graphs to pass in i18n context


- [#2230](#2230)
[`5fd3aa351`](5fd3aa3)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Add screen reader
support for pi-based numbers


- [#2224](#2224)
[`639eb089d`](639eb08)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Add minimal
instructions for how to interact with graph


- [#2205](#2205)
[`ae29e2b2f`](ae29e2b)
Thanks [@catandthemachines](https://github.com/catandthemachines)! -
Updating perseus analytics and events for better metrics.


- [#2232](#2232)
[`dc9989893`](dc99898)
Thanks [@beaesguerra](https://github.com/beaesguerra)! - Tooling:
Enabled jsx-a11y lint rules and disabled existing errors that were found


- [#2229](#2229)
[`91cd0c937`](91cd0c9)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Sinusoid - add
screen reader support


- [#2236](#2236)
[`df75123e5`](df75123)
Thanks [@nishasy](https://github.com/nishasy)! - [LX] Add hairlines when
Circle center has focus


- [#2243](#2243)
[`43005350f`](4300535)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Angle - Update
strings to reflect changes to SRUX specs


- [#2245](#2245)
[`037aaa2f4`](037aaa2)
Thanks [@catandthemachines](https://github.com/catandthemachines)! -
fixing bug in getWidgetSubTypeByWidgetId function.


- [#2213](#2213)
[`db9bc4fb6`](db9bc4f)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Angle - Add the
"interactive elements" overall graph description


- [#2212](#2212)
[`3ec6ec179`](3ec6ec1)
Thanks [@nishasy](https://github.com/nishasy)! - [LX] Add hairlines when
point has focus


- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`


- [#2221](#2221)
[`71329fe35`](71329fe)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Add aria-labels
for x- and y-axis labels


- [#2244](#2244)
[`c565e26d4`](c565e26)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Replace string-based function call with switch statement

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`dc9989893`](dc99898),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Minor Changes

- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`


- [#2234](#2234)
[`1ade12c18`](1ade12c)
Thanks [@handeyeco](https://github.com/handeyeco)! - Move splitters into
perseus-core, add splitPerseusItem

### Patch Changes

- [#2223](#2223)
[`f8a4becb0`](f8a4bec)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! -
[Bugfix] Interactive Graph crashes in editor when setting domain for
locked function


- [#2205](#2205)
[`ae29e2b2f`](ae29e2b)
Thanks [@catandthemachines](https://github.com/catandthemachines)! -
Updating perseus analytics and events for better metrics.


- [#2227](#2227)
[`ce320b496`](ce320b4)
Thanks [@benchristel](https://github.com/benchristel)! - Bugfix: Accept
'scientific' as a buttonSets value when parsing Expression widgets

## @khanacademy/[email protected]

### Patch Changes

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- [#2232](#2232)
[`dc9989893`](dc99898)
Thanks [@beaesguerra](https://github.com/beaesguerra)! - Tooling:
Enabled jsx-a11y lint rules and disabled existing errors that were found


- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- [#2223](#2223)
[`f8a4becb0`](f8a4bec)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! -
[Bugfix] Interactive Graph crashes in editor when setting domain for
locked function


- [#2230](#2230)
[`5fd3aa351`](5fd3aa3)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Add screen reader
support for pi-based numbers


- [#2232](#2232)
[`dc9989893`](dc99898)
Thanks [@beaesguerra](https://github.com/beaesguerra)! - Tooling:
Enabled jsx-a11y lint rules and disabled existing errors that were found


- [#2229](#2229)
[`91cd0c937`](91cd0c9)
Thanks [@nishasy](https://github.com/nishasy)! - [SR] Sinusoid - add
screen reader support


- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`


- [#2244](#2244)
[`c565e26d4`](c565e26)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Replace string-based function call with switch statement

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`91cede41f`](91cede4),
[`5fd3aa351`](5fd3aa3),
[`639eb089d`](639eb08),
[`edd34241e`](edd3424),
[`ae29e2b2f`](ae29e2b),
[`dc9989893`](dc99898),
[`91cd0c937`](91cd0c9),
[`df75123e5`](df75123),
[`43005350f`](4300535),
[`037aaa2f4`](037aaa2),
[`db9bc4fb6`](db9bc4f),
[`3ec6ec179`](3ec6ec1),
[`8e4cb7f53`](8e4cb7f),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`71329fe35`](71329fe),
[`c565e26d4`](c565e26),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

- [#2235](#2235)
[`ab2041897`](ab20418)
Thanks [@benchristel](https://github.com/benchristel)! - Remove unused
code, and export the `ParseFailureDetail` type from
`@khanacademy/perseus-core`

- Updated dependencies
\[[`f8a4becb0`](f8a4bec),
[`ae29e2b2f`](ae29e2b),
[`dc9989893`](dc99898),
[`ab2041897`](ab20418),
[`1ade12c18`](1ade12c),
[`ce320b496`](ce320b4)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]
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.

2 participants