Skip to content

Commit

Permalink
Knip 12: widget option refinement (#1782)
Browse files Browse the repository at this point in the history
## Summary:
`PerseusSequenceWidgetOptions` and `PerseusSimulatorWidgetOptions` are obsolete due to the deprecated stand-in.

LabelImage and Measurer weren't using their widget options type, but I think they should be (the other widgets do).

Author: handeyeco

Reviewers: jeremywiebe, benchristel, handeyeco

Required Reviewers:

Approved By: jeremywiebe, benchristel

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1782
  • Loading branch information
handeyeco authored Oct 22, 2024
1 parent f220366 commit ea7ede6
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 48 deletions.
5 changes: 5 additions & 0 deletions .changeset/honest-geckos-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Cleanup widget option types
20 changes: 1 addition & 19 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ export type PerseusMeasurerWidgetOptions = {
// The number of units to display on the ruler
rulerLength: number;
// Containing area [width, height]
box: ReadonlyArray<number>;
box: [number, number];
// Always false. Not used for this widget
static: boolean;
};
Expand Down Expand Up @@ -1200,24 +1200,6 @@ export type PerseusRadioChoice = {
widgets?: PerseusWidgetsMap;
};

export type PerseusSequenceWidgetOptions = {
// A list of Renderers to display in sequence
json: ReadonlyArray<PerseusRenderer>;
};

export type PerseusSimulatorWidgetOptions = {
// Translatable Text; The X Axis
xAxisLabel: string;
// Translatable Text; The Y Axis
yAxisLabel: string;
// Translatable Text; A lable to define the proportion of the simulation
proportionLabel: string;
// The type of simulation. options: "proportion", "percentage"
proportionOrPercentage: string;
// The number of times to run the simulation
numTrials: number;
};

export type PerseusSorterWidgetOptions = {
// Translatable Text; The correct answer (in the correct order). The user will see the cards in a randomized order.
correct: ReadonlyArray<string>;
Expand Down
18 changes: 5 additions & 13 deletions packages/perseus/src/widgets/label-image/label-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import Marker from "./marker";
import type {InteractiveMarkerType} from "./types";
import type {DependencyProps} from "../../dependencies";
import type {ChangeableProps} from "../../mixins/changeable";
import type {PerseusLabelImageWidgetOptions} from "../../perseus-types";
import type {APIOptions, Widget, WidgetExports} from "../../types";
import type {
PerseusLabelImageRubric,
Expand Down Expand Up @@ -67,23 +68,14 @@ type Point = {
y: number;
};

// TODO: should this be using WidgetProps / PerseusLabelImageWidgetOptions?
type LabelImageProps = ChangeableProps &
DependencyProps & {
DependencyProps &
// TODO: there's some weirdness in our types between
// PerseusLabelImageMarker and InteractiveMarkerType
Omit<PerseusLabelImageWidgetOptions, "markers"> & {
apiOptions: APIOptions;
// The list of possible answer choices.
choices: ReadonlyArray<string>;
// The question image properties.
imageAlt: string;
imageUrl: string;
imageWidth: number;
imageHeight: number;
// The list of label markers on the question image.
markers: ReadonlyArray<InteractiveMarkerType>;
// Whether multiple answer choices may be selected for markers.
multipleAnswers: boolean;
// Whether to hide answer choices from user instructions.
hideChoicesFromInstructions: boolean;
// Whether the question has been answered by the user.
questionCompleted: boolean;
// preferred placement for popover (preference, not MUST)
Expand Down
25 changes: 9 additions & 16 deletions packages/perseus/src/widgets/measurer/measurer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import GraphUtils from "../../util/graph-utils";
import noopValidator from "../__shared__/noop-validator";

import type {Coord} from "../../interactive2/types";
import type {APIOptions, Widget, WidgetExports} from "../../types";
import type {PerseusMeasurerWidgetOptions} from "../../perseus-types";
import type {Widget, WidgetExports, WidgetProps} from "../../types";
import type {Interval} from "../../util/interval";

const defaultImage = {
Expand All @@ -17,22 +18,14 @@ const defaultImage = {
left: 0,
} as const;

type Props = {
apiOptions: APIOptions;
box: [number, number];
image: {
url?: string;
top?: number;
left?: number;
};
showProtractor: boolean;
type Props = WidgetProps<
PerseusMeasurerWidgetOptions,
PerseusMeasurerWidgetOptions
> & {
// TODO: these don't show up anywhere else in code
// I'm guessing they could just be constants
protractorX: number;
protractorY: number;
showRuler: boolean;
rulerLabel: string;
rulerTicks: number;
rulerPixels: number;
rulerLength: number;
};

type DefaultProps = {
Expand All @@ -51,7 +44,7 @@ type DefaultProps = {
class Measurer extends React.Component<Props> implements Widget {
static defaultProps: DefaultProps = {
box: [480, 480],
image: {},
image: defaultImage,
showProtractor: true,
protractorX: 7.5,
protractorY: 0.5,
Expand Down

0 comments on commit ea7ede6

Please sign in to comment.