Update Matrix to render with answerless data #2387
Conversation
|
Size Change: +23 B (0%) Total Size: 466 kB
ℹ️ View Unchanged
|
…t to render with answerless data. Adds tests and stories for answerless rendering.
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (ed16766) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2387If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.js -t PR2387 |
| type RenderProps = MatrixPublicWidgetOptions & { | ||
| answers: ReadonlyArray<ReadonlyArray<string>>; | ||
| }; |
There was a problem hiding this comment.
Not sure if this should be up with the other props. ExternalProps doesn't have the same optional properties as MatrixPublicWidgetOptions, so I didn't think I should add RenderProps to it. The typing for answers is a little strange; transform converts it to a string, so the matrix gets rendered with an array of strings, but there's other logic using it as numbers. The matrix editor creates the array with numbers, but then all the interaction is with strings, until you get back to scoring.
There was a problem hiding this comment.
I belive the difference between RenderProps and ExternalProps is that RenderProps is supposed to be what appears inside the component (after defaultProps have been applied) and ExternalProps is what callers of this component need to pass in. But TBH I am a little confused about why we need both types here. It seems like maybe we could define type RenderProps = ExternalProps for this component? I will look into this more after lunch.
There was a problem hiding this comment.
tbh I was very confused by props in Table too. I think for some of these older widgets, the architecture wasn't as clear.
There's some info here: https://github.com/Khan/perseus/blob/main/docs/architecture.md
I think it's:
PerseusMatrixWidgetOptions: all the data we store about the specific widgetMatrixPublicWidgetOptions: all the answerless data for the pre-scoring renderingRenderProps: the output of thetransformwhich takesPerseusMatrixWidgetOptions(and in the futureMatrixPublicWidgetOptions) as inputExternalProps: wrapsRenderPropswith additional stuff fromWidgetOptions*DefaultProps: is the defaults for things that are optional inExternalPropsProps: is the combination ofExternalPropsandDefaultProps- it's everything inExternalPropsexcept optional fields are filled in fromDefaultProps
*WidgetOptions is its own thing:
/**
* The type representing the common structure of all widget's options. The
* `Options` generic type represents the widget-specific option data.
*/
export type WidgetOptions<Type extends string, Options> = {
// The "type" of widget which will define what the Options field looks like
type: Type;
// Whether this widget is displayed with the values and is immutable. For display only
// NOTE: perseus_data.go says this is required even though it isn't necessary.
static?: boolean;
// Whether a widget is scored. Usually true except for IFrame widgets (deprecated)
// Default: true
graded?: boolean;
// The HTML alignment of the widget. "default" or "block"
// NOTE: perseus_data.go says this is required even though it isn't necessary.
alignment?: string;
// Options specific to the type field of the widget. See Perseus*WidgetOptions for more details
options: Options;
// Only used by interactive child widgets (line, point, etc) to identify the components
// NOTE: perseus_data.go says this is required even though it isn't necessary.
key?: number | null;
// The version of the widget data spec. Used to differentiate between newer and older content data.
// NOTE: perseus_data.go says this is required even though it isn't necessary.
version?: Version;
};There was a problem hiding this comment.
Alright, I didn't make any changes to ExternalProps or Props. Just added in the RenderProps type. I did notice that getUserInput returns an object with an answers property, but changing the naming there requires a lot of changes, so if that's important, might do that in another PR. Otherwise, will be landing this tomorrow :)
| type RenderProps = MatrixPublicWidgetOptions & { | ||
| answers: ReadonlyArray<ReadonlyArray<string>>; | ||
| }; |
There was a problem hiding this comment.
I belive the difference between RenderProps and ExternalProps is that RenderProps is supposed to be what appears inside the component (after defaultProps have been applied) and ExternalProps is what callers of this component need to pass in. But TBH I am a little confused about why we need both types here. It seems like maybe we could define type RenderProps = ExternalProps for this component? I will look into this more after lunch.
| type RenderProps = MatrixPublicWidgetOptions & { | ||
| answers: ReadonlyArray<ReadonlyArray<string>>; | ||
| }; |
There was a problem hiding this comment.
tbh I was very confused by props in Table too. I think for some of these older widgets, the architecture wasn't as clear.
There's some info here: https://github.com/Khan/perseus/blob/main/docs/architecture.md
I think it's:
PerseusMatrixWidgetOptions: all the data we store about the specific widgetMatrixPublicWidgetOptions: all the answerless data for the pre-scoring renderingRenderProps: the output of thetransformwhich takesPerseusMatrixWidgetOptions(and in the futureMatrixPublicWidgetOptions) as inputExternalProps: wrapsRenderPropswith additional stuff fromWidgetOptions*DefaultProps: is the defaults for things that are optional inExternalPropsProps: is the combination ofExternalPropsandDefaultProps- it's everything inExternalPropsexcept optional fields are filled in fromDefaultProps
*WidgetOptions is its own thing:
/**
* The type representing the common structure of all widget's options. The
* `Options` generic type represents the widget-specific option data.
*/
export type WidgetOptions<Type extends string, Options> = {
// The "type" of widget which will define what the Options field looks like
type: Type;
// Whether this widget is displayed with the values and is immutable. For display only
// NOTE: perseus_data.go says this is required even though it isn't necessary.
static?: boolean;
// Whether a widget is scored. Usually true except for IFrame widgets (deprecated)
// Default: true
graded?: boolean;
// The HTML alignment of the widget. "default" or "block"
// NOTE: perseus_data.go says this is required even though it isn't necessary.
alignment?: string;
// Options specific to the type field of the widget. See Perseus*WidgetOptions for more details
options: Options;
// Only used by interactive child widgets (line, point, etc) to identify the components
// NOTE: perseus_data.go says this is required even though it isn't necessary.
key?: number | null;
// The version of the widget data spec. Used to differentiate between newer and older content data.
// NOTE: perseus_data.go says this is required even though it isn't necessary.
version?: Version;
};Specifically item to renderer as a renderer is what's being created
Summary:
Updates the Matrix widget to render using MatrixPublicWidgetOptions instead of the full widget options. Adds a Storybook story showing the widget can render, is interactive, and is scorable when rendered with answerless data. Answerful data is still required for the actual scoring. Tests showing this functionality are also included.
The render section of Matrix still references
this.props.answers, but thetransformfunction takes a widgetOptions object without answers and adds in an answers array shaped after the board size. The default props are set for an empty nested array, so I think that's all good. The answers field is also what carries the user input.Issue: LEMS-2975
Test plan: