-
Notifications
You must be signed in to change notification settings - Fork 350
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
Migrate PerseusWidgetsMap to use a merged interface to represent all available widgets #1936
Changes from 4 commits
b8eb8cd
80721e7
ee5b07f
5d5e415
03ee194
6a590e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@khanacademy/perseus": minor | ||
--- | ||
|
||
Changes the PerseusWidgetsMap to be extensible so that widgets can be registered outside of Perseus and still have full type safety. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,72 +11,89 @@ export type Size = [number, number]; | |
export type CollinearTuple = [vec.Vector2, vec.Vector2]; | ||
export type ShowSolutions = "all" | "selected" | "none"; | ||
|
||
/** | ||
* Our core set of Perseus widgets. | ||
* | ||
* This interface is the basis for "registering" all Perseus widget types. | ||
* There should be one key/value pair for each supported widget. If you create | ||
* a new widget, an entry should be added to this interface. Note that this | ||
* only registers the widget options type, you'll also need to register the | ||
* widget so that it's available at runtime (@see | ||
* {@link file://./widgets.ts#registerWidget}). | ||
* | ||
* Importantly, the key is not important and is not used anywhere outside of | ||
* this interface so it is arbitrary. | ||
* | ||
* If you define the widget outside of this package, you can still add the new | ||
* widget to this interface by writing the following in that pacakge that | ||
jeremywiebe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* contains the widget. TypeScript will merge that definition of the | ||
* `PerseusWidgets` with the one defined below. | ||
* | ||
* ```typescript | ||
* declare module "@khanacademy/perseus" { | ||
* interface PerseusWidgetTypes { | ||
* Awesomeness: MyAwesomeNewWidget; | ||
* } | ||
* } | ||
* | ||
* type MyAwesomeNewWidget = WidgetOptions<'awesomeness', MyAwesomeNewWidgetOptions>; | ||
* ``` | ||
* | ||
* This interface can be extended through the magic of TypeScript "Declaration | ||
* merging". Specifically, we augment this module and extend this interface. | ||
* | ||
* @see {@link https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation} | ||
*/ | ||
export interface PerseusWidgetTypes { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although this looks like the same hard-coded list of widgets as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a step in the right direction, I just wish we could get to a place where the Renderer-layer of Perseus is completely unaware of the Widget-layer data which would mean finding ways to eliminate these type of widget union/map things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed! I've spent a decent amount of time thinking about how we could make the lower level types opaque to the Renderers, I don't have a solution yet. |
||
Categorizer: CategorizerWidget; | ||
CSProgram: CSProgramWidget; | ||
Definition: DefinitionWidget; | ||
Dropdown: DropdownWidget; | ||
Explanation: ExplanationWidget; | ||
Expression: ExpressionWidget; | ||
Grapher: GrapherWidget; | ||
GradedGroupSet: GradedGroupSetWidget; | ||
GradedGroup: GradedGroupWidget; | ||
Group: GroupWidget; | ||
IFrame: IFrameWidget; | ||
Image: ImageWidget; | ||
InputNumber: InputNumberWidget; | ||
Interaction: InteractionWidget; | ||
InteractiveGraph: InteractiveGraphWidget; | ||
LabelImage: LabelImageWidget; | ||
Matcher: MatcherWidget; | ||
Matrix: MatrixWidget; | ||
Measurer: MeasurerWidget; | ||
MoleculeRenderer: MoleculeRendererWidget; | ||
NumberLine: NumberLineWidget; | ||
NumericInput: NumericInputWidget; | ||
Orderer: OrdererWidget; | ||
PassageRefTarget: RefTargetWidget; | ||
PassageRef: PassageRefWidget; | ||
Passage: PassageWidget; | ||
PhetSimulation: PhetSimulationWidget; | ||
Python: PythonProgramWidget; | ||
Plotter: PlotterWidget; | ||
Radio: RadioWidget; | ||
Sorter: SorterWidget; | ||
Table: TableWidget; | ||
Video: VideoWidget; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not related to your PR and no action needed: can we leverage this or something like this to get rid of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. This interface is a registry of widget option types whereas the things you mentioned are the "runtime registries". We could certainly merge all those three items together though. |
||
} | ||
|
||
/** | ||
* A map of widget IDs to widget options. This is most often used as the type | ||
* for a set of widgets defined in a `PerseusItem` but can also be useful to | ||
* represent a function parameter where only `widgets` from a `PerseusItem` are | ||
* needed. Today Widget IDs are made up of the widget type and an incrementing | ||
* integer (eg. `interactive-graph 1` or `radio 3`). It is suggested to avoid | ||
* reading/parsing the widget id to derive any information from it, except in | ||
* the case of this map. | ||
* | ||
* @see {@link PerseusWidgetTypes} additional widgets can be added to this map type | ||
* by augmenting the PerseusWidgetTypes with new widget types! | ||
*/ | ||
export type PerseusWidgetsMap = { | ||
[key in `categorizer ${number}`]: CategorizerWidget; | ||
} & { | ||
[key in `cs-program ${number}`]: CSProgramWidget; | ||
} & { | ||
[key in `definition ${number}`]: DefinitionWidget; | ||
} & { | ||
[key in `dropdown ${number}`]: DropdownWidget; | ||
} & { | ||
[key in `explanation ${number}`]: ExplanationWidget; | ||
} & { | ||
[key in `expression ${number}`]: ExpressionWidget; | ||
} & { | ||
[key in `grapher ${number}`]: GrapherWidget; | ||
} & { | ||
[key in `group ${number}`]: GroupWidget; | ||
} & { | ||
[key in `graded-group ${number}`]: GradedGroupWidget; | ||
} & { | ||
[key in `graded-group-set ${number}`]: GradedGroupSetWidget; | ||
} & { | ||
[key in `iframe ${number}`]: IFrameWidget; | ||
} & { | ||
[key in `image ${number}`]: ImageWidget; | ||
} & { | ||
[key in `input-number ${number}`]: InputNumberWidget; | ||
} & { | ||
[key in `interaction ${number}`]: InteractionWidget; | ||
} & { | ||
[key in `interactive-graph ${number}`]: InteractiveGraphWidget; | ||
} & { | ||
[key in `label-image ${number}`]: LabelImageWidget; | ||
} & { | ||
[key in `matcher ${number}`]: MatcherWidget; | ||
} & { | ||
[key in `matrix ${number}`]: MatrixWidget; | ||
} & { | ||
[key in `measurer ${number}`]: MeasurerWidget; | ||
} & { | ||
[key in `molecule-renderer ${number}`]: MoleculeRendererWidget; | ||
} & { | ||
[key in `number-line ${number}`]: NumberLineWidget; | ||
} & { | ||
[key in `numeric-input ${number}`]: NumericInputWidget; | ||
} & { | ||
[key in `orderer ${number}`]: OrdererWidget; | ||
} & { | ||
[key in `passage ${number}`]: PassageWidget; | ||
} & { | ||
[key in `passage-ref ${number}`]: PassageRefWidget; | ||
} & { | ||
[key in `passage-ref-target ${number}`]: PassageRefWidget; | ||
} & { | ||
[key in `phet-simulation ${number}`]: PhetSimulationWidget; | ||
} & { | ||
[key in `plotter ${number}`]: PlotterWidget; | ||
} & { | ||
[key in `python-program ${number}`]: PythonProgramWidget; | ||
} & { | ||
[key in `radio ${number}`]: RadioWidget; | ||
} & { | ||
[key in `sorter ${number}`]: SorterWidget; | ||
} & { | ||
[key in `table ${number}`]: TableWidget; | ||
} & { | ||
[key in `video ${number}`]: VideoWidget; | ||
[Property in keyof PerseusWidgetTypes as `${PerseusWidgetTypes[Property]["type"]} ${number}`]: PerseusWidgetTypes[Property]; | ||
jeremywiebe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know that these will be used in Webapp or are we preemptively exporting them? If not, I wonder if it would be better to keep them internal until we have a use for them externally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PerseusWidgetTypes
needs to be exported so that it can be extended from anywhere outside of Perseus. @kevinb-khan has already had a use case for this while building a hackathon widget inside of webapp.WidgetOptions
is a very useful helper type for defining a widget's widget options structure.Both are forward-looking, but it seemed reasonable to export them as part of this PR given the "widgets can be created outside of the @khanacademy/perseus" package nature.