Skip to content

Commit

Permalink
Improve some comments (#1934)
Browse files Browse the repository at this point in the history
## Summary:

As it says, just some comments that I wanted to improve as I was working on SSS.

Issue: "none"

## Test plan:

Read the comments.

Author: jeremywiebe

Reviewers: SonicScrewdriver, jeremywiebe, nishasy, benchristel, handeyeco, catandthemachines, anakaren-rojas, mark-fitzgerald

Required Reviewers:

Approved By: SonicScrewdriver

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1934
  • Loading branch information
jeremywiebe authored Dec 3, 2024
1 parent e3062b3 commit 129adeb
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .changeset/tall-pianos-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Improve comments on some Perseus types
37 changes: 27 additions & 10 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export type PerseusArticle = PerseusRenderer | ReadonlyArray<PerseusRenderer>;
* A "MultiItem" is an advanced Perseus item. It is rendered by the
* `MultiRenderer` and you can control the layout of individual parts of the
* item.
*
* @deprecated MultiItem support is slated for removal in a future Perseus
* release.
*/
export type MultiItem = {
// Multi-item should only show up in Test Prep content and it is a variant of a PerseusItem
Expand All @@ -128,20 +131,28 @@ export type Version = {
};

export type PerseusRenderer = {
// Translatable Markdown content to be rendered. May include references to
// widgets (as [[☃ widgetName]]) or images (as ![image text](imageUrl)).
// For each image found in this content, there can be an entry in the
// `images` dict (below) with the key being the image's url which defines
// additional attributes for the image.
/**
* Translatable Markdown content to be rendered. May include references to
* widgets (as [[☃ widgetName]]) or images (as ![image text](imageUrl)).
* For each image found in this content, there can be an entry in the
* `images` dict (below) with the key being the image's url which defines
* additional attributes for the image.
*/
content: string;
// A dictionary of {[widgetName]: Widget} to be referenced from the content field
/**
* A dictionary of {[widgetName]: Widget} to be referenced from the content
* field.
*/
widgets: PerseusWidgetsMap;
// Used in the PerseusGradedGroup widget. A list of "tags" that are keys that represent other content in the system. Not rendered to the user.
// Used in the PerseusGradedGroup widget. A list of "tags" that are keys
// that represent other content in the system. Not rendered to the user.
// NOTE: perseus_data.go says this is required even though it isn't necessary.
metadata?: ReadonlyArray<string>;
// A dictionary of {[imageUrl]: PerseusImageDetail}.
/**
* A dictionary of {[imageUrl]: PerseusImageDetail}.
*/
images: {
[key: string]: PerseusImageDetail;
[imageUrl: string]: PerseusImageDetail;
};
};

Expand Down Expand Up @@ -183,6 +194,10 @@ export const ItemExtras = [
] as const;
export type PerseusAnswerArea = Record<(typeof ItemExtras)[number], boolean>;

/**
* 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;
Expand Down Expand Up @@ -310,7 +325,9 @@ export type PerseusWidget =
| VideoWidget
| AutoCorrectWidget;

// A background image applied to various widgets.
/**
* A background image applied to various widgets.
*/
export type PerseusImageBackground = {
// The URL of the image
url: string | null | undefined;
Expand Down
6 changes: 6 additions & 0 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,12 @@ class Renderer
return state;
};

/**
* Returns an array of widget ids that are empty (meaning widgets where the
* learner has not interacted with the widget yet or has not filled in all
* fields). For example, the `interactive-graph` widget is considered
* empty if the graph is in the starting state.
*/
emptyWidgets(): ReadonlyArray<string> {
return emptyWidgetsFunctional(
this.state.widgetInfo,
Expand Down
92 changes: 60 additions & 32 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ export interface Widget {
getDOMNodeForPath?: (path: FocusPath) => Element | Text | null;
deselectIncorrectSelectedChoices?: () => void;

/**
* Returns widget state that can be passed back to `restoreSerializedState`
* to put the widget back into exactly the same state. If the widget does
* not implement this function, the renderer simply returns all of the
* widget's props.
*/
// TODO(jeremy): I think this return value is wrong. The widget
// getSerializedState should just return _its_ serialized state, not a
// key/value list of all widget states (i think!)
// Returns widget state that can be passed back to `restoreSerializedState`
// to put the widget back into exactly the same state. If the widget does
// not implement this function, the renderer simply returns all of the
// widget's props.
getSerializedState?: () => SerializedState; // SUSPECT,
restoreSerializedState?: (props: any, callback: () => void) => any;

Expand Down Expand Up @@ -459,15 +461,17 @@ type InitialRequestUrlInterface = {

export type VideoKind = "YOUTUBE_ID" | "READABLE_ID";

// An object for dependency injection, to allow different clients
// to provide different methods for logging, translation, network
// requests, etc.
//
// NOTE: You should avoid adding new dependencies here as this type was added
// as a quick fix to get around the fact that some of the dependencies Perseus
// needs are used in places where neither `APIOptions` nor a React Context
// could be used. Aim to shrink the footprint of PerseusDependencies and try to
// use alternative methods where possible.
/**
* An object for dependency injection, to allow different clients
* to provide different methods for logging, translation, network
* requests, etc.
*
* NOTE: You should avoid adding new dependencies here as this type was added
* as a quick fix to get around the fact that some of the dependencies Perseus
* needs are used in places where neither `APIOptions` nor a React Context
* could be used. Aim to shrink the footprint of PerseusDependencies and try to
* use alternative methods where possible.
*/
export type PerseusDependencies = {
// JIPT
JIPT: JIPT;
Expand Down Expand Up @@ -563,9 +567,11 @@ export type Alignment =

type WidgetOptions = any;

// A transform that maps the WidgetOptions (sometimes referred to as
// EditorProps) to the props used to render the widget. Often this is an
// identity transform.
/**
* A transform that maps the WidgetOptions (sometimes referred to as
* EditorProps) to the props used to render the widget. Often this is an
* identity transform.
*/
// TODO(jeremy): Make this generic so that the WidgetOptions and output type
// become strongly typed.
export type WidgetTransform = (
Expand Down Expand Up @@ -602,11 +608,12 @@ export type WidgetExports<
/** Supresses widget from showing up in the dropdown in the content editor */
hidden?: boolean;
/**
The widget version. Any time the _major_ version changes, the widget
should provide a new entry in the propUpgrades map to migrate from the
older version to the current (new) version. Minor version changes must
be backwards compatible with previous minor versions widget options.
This key defaults to `{major: 0, minor: 0}` if not provided.
* The widget version. Any time the _major_ version changes, the widget
* should provide a new entry in the propUpgrades map to migrate from the
* older version to the current (new) version. Minor version changes must
* be backwards compatible with previous minor versions widget options.
*
* This key defaults to `{major: 0, minor: 0}` if not provided.
*/
version?: Version;
supportedAlignments?: ReadonlyArray<Alignment>;
Expand All @@ -617,27 +624,40 @@ export type WidgetExports<

traverseChildWidgets?: any; // (Props, traverseRenderer) => NewProps,

/** transforms the widget options to the props used to render the widget */
/**
* Transforms the widget options to the props used to render the widget.
*/
transform?: WidgetTransform;
/** transforms the widget options to the props used to render the widget for
static renders */
/**
* Transforms the widget options to the props used to render the widget for
* static renders.
*/
staticTransform?: WidgetTransform; // this is a function of some sort,

/**
* A function that scores user input (the guess) for the widget.
*/
scorer?: WidgetScorerFunction;

getOneCorrectAnswerFromRubric?: (
rubric: Rubric,
) => string | null | undefined;

/**
A map of major version numbers (as a string, eg "1") to a function that
migrates from the _previous_ major version.
Example:
propUpgrades: {'1': (options) => ({...options})}
would migrate from major version 0 to 1.
*/
* A map of major version numbers (as a string, eg "1") to a function that
* migrates from the _previous_ major version.
*
* Example:
* ```
* propUpgrades: {'1': (options) => ({...options})}
* ```
*
* This configuration would migrate options from major version 0 to 1.
*/
propUpgrades?: {
[key: string]: (arg1: any) => any;
}; // OldProps => NewProps,
// OldProps => NewProps,
[targetMajorVersion: string]: (arg1: any) => any;
};
}>;

export type FilterCriterion =
Expand All @@ -648,6 +668,14 @@ export type FilterCriterion =
widget?: Widget | null | undefined,
) => boolean);

/**
* The full set of props provided to all widgets when they are rendered. The
* `RenderProps` generic argument are the widget-specific props that originate
* from the stored PerseusItem. Note that they may not match the serialized
* widget options exactly as they are the result of running the options through
* any `propUpgrades` the widget defines as well as its `transform` or
* `staticTransform` functions (depending on the options `static` flag).
*/
// NOTE: Rubric should always be the corresponding widget options type for the component.
// TODO: in fact, is it really the rubric? WidgetOptions is what we use to configure the widget
// (which is what this seems to be for)
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfig-shared.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// This file is used by the tsconfig.json files in each package.
// This file is used by the tsconfig-build.json files in each package.
/* Visit https://aka.ms/tsconfig to read more about this file */
{
"exclude": [
Expand Down

0 comments on commit 129adeb

Please sign in to comment.