diff --git a/.changeset/popular-cobras-accept.md b/.changeset/popular-cobras-accept.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/popular-cobras-accept.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.eslintrc.js b/.eslintrc.js index 0e609848363..cf0068e7cb6 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -350,7 +350,6 @@ module.exports = { "react/sort-comp": [ "error", { - // TODO(kevinb): specify where "constructor" should go order: [ "type-annotations", "static-variables", diff --git a/config/test/test.config.js b/config/test/test.config.js index 65ee6b644c5..f253b17f556 100644 --- a/config/test/test.config.js +++ b/config/test/test.config.js @@ -103,5 +103,6 @@ module.exports = { // TODO(LEMS-2858) We turn off Prettier as Prettier v3 is incompatible with // Jest v29. Once they release Jest v30 we can switch to that: // https://github.com/jestjs/jest/issues/14305 + // NOTE: Jest v30 has been released. prettierPath: null, }; diff --git a/packages/kas/src/__tests__/comparing.test.ts b/packages/kas/src/__tests__/comparing.test.ts index 1eaa8ea0b4a..bca948f0988 100644 --- a/packages/kas/src/__tests__/comparing.test.ts +++ b/packages/kas/src/__tests__/comparing.test.ts @@ -163,8 +163,6 @@ describe("comparing", () => { expect("x^2+y^2=r^2").toEqualExpr("r^2=x^2+y^2"); expect("23^1.5=110.304").toEqualExpr("110.304=23^1.5"); - // TODO(alex): make sure that I have both positive and negative - // test cases for all functionality expect("6.12*10^-2").toEqualExpr("6.12*10^-2"); expect("6.12*10^-2").not.toEqualExpr("6.12*10^-6"); @@ -198,7 +196,6 @@ describe("comparing", () => { expect("sin(500pi x)").not.toEqualExpr("sin(pi x)"); // Check that floating point error isn't killing us - // TODO(jack): These don't seem to test much; make better tests expect("0").toEqualExpr("sin(7pi)"); expect("sin(7pi)").toEqualExpr("0"); expect("0").toEqualExpr("sin(500pi)"); @@ -293,8 +290,6 @@ describe("comparing", () => { // to isolate `f(x)` in each expression and then check that the // expressions that they're equal to are equal. In this case that // would be `sin^2(x)+cos^2(x)` and `1`. - // TODO(TP-11651): Update compare to isolate functions on wide side - // before comparing expressions on the other side. // expect("f(x) = sin^2(x)+cos^2(x)").toEqualExpr("f(x) = 1"); expect("f(x) = ln|x|+c").toEqualExpr("f(x)-ln|x|-c = 0"); diff --git a/packages/kas/src/__tests__/index.test.ts b/packages/kas/src/__tests__/index.test.ts index b4a5793f41f..864fa6837a4 100644 --- a/packages/kas/src/__tests__/index.test.ts +++ b/packages/kas/src/__tests__/index.test.ts @@ -263,7 +263,6 @@ describe("KAS", () => { expect("ln(x+y)").toBeSimplified(); // Will only expand logarithms if leads to a simpler expression - // TODO(alex): Combine all simplify(assert, ) and isSimplified(assert, ) tests! expect("ln(x/y)").toBeSimplified(); expect("ln(x/y)+ln(y)").not.toBeSimplified(); }); @@ -329,7 +328,6 @@ describe("KAS", () => { expect("y=(x+1)/(x+2)").toBeSimplified(); expect("y/(x+1)=1/(x+2)").toBeSimplified(); - // TODO(alex): Combine all isSimplified(assert, ) and simplify(assert, ) tests! expect("y=(x+1)/(2x+2)").not.toBeSimplified(); expect("y=1/2").toBeSimplified(); diff --git a/packages/kas/src/__tests__/parsing.test.ts b/packages/kas/src/__tests__/parsing.test.ts index 6753d6e8851..0c0786e6705 100644 --- a/packages/kas/src/__tests__/parsing.test.ts +++ b/packages/kas/src/__tests__/parsing.test.ts @@ -290,8 +290,6 @@ describe("parsing", () => { expect("|x|").toParseAs("abs(x)"); expect("||x||").toParseAs("abs(abs(x))"); - // TODO(alex): fix the below so it doesn't require an * - // may require own lexer/preprocessor expect("|x|*|y|").toParseAs("abs(x)*abs(y)"); expect("\\abs(x)").toParseAs("abs(x)"); diff --git a/packages/kas/src/nodes.ts b/packages/kas/src/nodes.ts index 54646b2422a..715270cb116 100644 --- a/packages/kas/src/nodes.ts +++ b/packages/kas/src/nodes.ts @@ -561,7 +561,6 @@ abstract class Expr { return new Mul(NumOne, this); } - // TODO(alex): rename to isDefinitePositive or similar? // return whether this expression is 100% positive isPositive(): boolean { throw new Error( @@ -571,7 +570,6 @@ abstract class Expr { ); } - // TODO(alex): rename to hasNegativeSign or similar? // return whether this expression has a negative sign isNegative() { return false; @@ -1388,7 +1386,6 @@ export class Mul extends Seq { if (expr instanceof Num && expr.n > 0) { // e.g. - 2 -> -2 var negated = expr.negate(); - // TODO(alex): rework hint system so that this isn't necessary negated.hints = expr.hints; return negated.addHint(hint); } else if (expr instanceof Mul) { @@ -2509,7 +2506,6 @@ export class Trig extends Expr { } } - // TODO(alex): does every new node type need to redefine these? needsExplicitMul() { return false; } diff --git a/packages/math-input/src/components/input/__tests__/mathquill.test.ts b/packages/math-input/src/components/input/__tests__/mathquill.test.ts index cb0c71b5e5a..ca2a3150167 100644 --- a/packages/math-input/src/components/input/__tests__/mathquill.test.ts +++ b/packages/math-input/src/components/input/__tests__/mathquill.test.ts @@ -28,8 +28,6 @@ describe("MathQuill", () => { document.body.removeChild(span); }); - // TODO(charlie): Add tests for "FRAC_EXCLUSIVE" (the mixed-number - // fraction key). describe("Fraction Bar", () => { it("should work with no content", () => { mathField.pressKey("FRAC_INCLUSIVE"); @@ -166,14 +164,6 @@ describe("MathQuill", () => { mathField.pressKey("EXP"); expect(mathField.getContent()).toEqual("35x^{ }"); }); - - // TODO(kevinb): makes the expression an exponent when it shouldn't - it.skip("should work on a selected expression", () => { - mathField.setContent("35x+5"); - mathField.selectAll(); - mathField.pressKey("EXP"); - expect(mathField.getContent()).toEqual("\\left(35x+5\\right)^{ }"); - }); }); describe("Square Root", () => { @@ -207,14 +197,6 @@ describe("MathQuill", () => { mathField.pressKey("RADICAL"); expect(mathField.getContent()).toEqual("35x^{2}\\sqrt[]{}"); }); - - it.skip("should work on a selected expression", () => { - mathField.setContent("35x+5"); - mathField.selectAll(); - mathField.pressKey("RADICAL"); - // TODO(kevinb): check cursor location - expect(mathField.getContent()).toEqual("\\sqrt[ ]{35x+5}"); - }); }); describe("Log", () => { @@ -279,7 +261,6 @@ describe("MathQuill", () => { expect(mathField.getContent()).toEqual("35x^{2}"); }); - // TODO(kevinb) math isn't selected it("should select a fraction when deleting from outside of it", () => { const expr = "\\frac{35x+5}{x^{2}}"; mathField.setContent(expr); @@ -314,15 +295,6 @@ describe("MathQuill", () => { expect(mathField.getContent()).toEqual("\\left(35x+\\right)"); }); - // TODO(kevinb) fix this behavior so that we delete the exponent too - it.skip("should not delete squared exponents", () => { - mathField.setContent("35x^{2}"); - mathField.pressKey("BACKSPACE"); - expect(mathField.getContent()).toEqual("35x^{2}"); - mathField.pressKey("BACKSPACE"); - expect(mathField.getContent()).toEqual("35x^{ }"); - }); - it("should not delete non-square exponents", () => { mathField.setContent("35x^5"); mathField.pressKey("BACKSPACE"); @@ -643,10 +615,4 @@ describe("MathQuill", () => { expect(cursor[MQ.R].ctrlSeq).toEqual("5"); }); }); - - describe.skip("Jump out", () => { - // TODO(charlie): Write extensive tests for the 'Jump out' behavior. - }); - - describe.skip("Equals =, !=, <, <=, >, >=", () => {}); }); diff --git a/packages/math-input/src/components/input/cursor-contexts.ts b/packages/math-input/src/components/input/cursor-contexts.ts index 7fac48ca3f1..44c9524435d 100644 --- a/packages/math-input/src/components/input/cursor-contexts.ts +++ b/packages/math-input/src/components/input/cursor-contexts.ts @@ -4,10 +4,6 @@ * for the `BEFORE_FRACTION` context), and then at its direct parent. Though a * cursor could in theory be nested in multiple contexts, we only care about the * immediate context. - * - * TODO(charlie): Add a context to represent being inside of a radical. Right - * now, we show the dismiss button rather than allowing the user to jump out of - * the radical. */ export enum CursorContext { diff --git a/packages/math-input/src/components/input/math-input.tsx b/packages/math-input/src/components/input/math-input.tsx index 931ea38be43..82436a523d4 100644 --- a/packages/math-input/src/components/input/math-input.tsx +++ b/packages/math-input/src/components/input/math-input.tsx @@ -98,11 +98,6 @@ class MathInput extends React.Component { this.context.locale, { onCursorMove: (cursor: Cursor) => { - // TODO(charlie): It's not great that there is so much coupling - // between this keypad and the input behavior. We should wrap - // this `MathInput` component in an intermediary component - // that translates accesses on the keypad into vanilla props, - // to make this input keypad-agnostic. this.props.keypadElement?.setCursor(cursor); }, }, @@ -146,7 +141,6 @@ class MathInput extends React.Component { // below the keypad is that the keypad may be anchored above // the 'Check answer' bottom bar, in which case, we don't want // to dismiss the keypad on check. - // TODO(charlie): Inject this logic. if (!this._container.contains(evt.target)) { let touchDidStartInOrBelowKeypad = false; if ( @@ -184,11 +178,6 @@ class MathInput extends React.Component { this.blurOnTouchEndOutside = (evt) => { // If the user didn't scroll, blur the input. - // TODO(charlie): Verify that the touch that ended actually started - // outside the keypad. Right now, you can touch down on the keypad, - // touch elsewhere, release the finger on the keypad, and trigger a - // dismissal. This code needs to be generalized to handle - // multi-touch. if (this.state.focused && this.didTouchOutside && !this.didScroll) { this.blur(); this.mathField.blur(); @@ -392,12 +381,6 @@ class MathInput extends React.Component { // Android Browser 4.3. setTimeout(() => { if (this._isMounted) { - // TODO(benkomalo): the keypad is animating at this point, - // so we can't call _cacheKeypadBounds(), even though - // it'd be nice to do so. It should probably be the case - // that the higher level controller tells us when the - // keypad is settled (then scrollIntoView wouldn't have - // to make assumptions about that either). const maybeKeypadNode = this.props.keypadElement?.getDOMNode(); scrollIntoView(this._container, maybeKeypadNode); @@ -506,7 +489,6 @@ class MathInput extends React.Component { // Since we're doing three hit tests per loop it's possible that // we hit multiple leaf nodes at the same time. In this case we // we prefer the DOMNode with the most hits. - // TODO(kevinb) consider preferring nodes hit by [x, y]. for (const [id, count] of entries(counts)) { if (count > max) { max = count; @@ -789,10 +771,6 @@ class MathInput extends React.Component { handle: { animateIntoPosition: false, visible: true, - // TODO(charlie): Use clientX and clientY to avoid the need for - // scroll offsets. This likely also means that the cursor - // detection doesn't work when scrolled, since we're not - // offsetting those values. x: this._constrainToBound( relativeX, 0, @@ -885,9 +863,6 @@ class MathInput extends React.Component { if (mathQuillKey) { this.mathField.pressKey(mathQuillKey); - // TODO(diedra): If the new value being added is off-screen to the right - // due to the max-width of the text box, scroll the box to show the newest - // value const value = this.mathField.getContent(); if (this.props.value !== value) { this.mathField.setContent(this.props.value); @@ -898,7 +873,6 @@ class MathInput extends React.Component { }; getBorderWidthPx: () => number = () => { - // TODO(diedra): Move these to the common style package. const normalBorderWidthPx = 1; const focusedBorderWidthPx = 2; @@ -958,8 +932,6 @@ class MathInput extends React.Component { // We added the tapping instructions because there is currently a bug where // Android users need to use two fingers to tap the input field to make the // keyboard appear. It should only require one finger, which is how iOS works. - // TODO(diedra): Fix the bug that is causing Android to require a two finger tap - // to the open the keyboard, and then remove the second half of this label. const ariaLabel = this.context.strings.mathInputBox + " " + diff --git a/packages/math-input/src/components/input/mathquill-instance.ts b/packages/math-input/src/components/input/mathquill-instance.ts index 43e6d1ff7b9..79d1ffc1b7a 100644 --- a/packages/math-input/src/components/input/mathquill-instance.ts +++ b/packages/math-input/src/components/input/mathquill-instance.ts @@ -79,12 +79,6 @@ const createBaseConfig = (): MathFieldConfig => ({ // if you're in one, but moves focus to the next input if you're // not. Spaces (with this option enabled) are just ignored in the // latter case. - // - // TODO(alex): In order to allow inputting mixed numbers, we will - // have to accept spaces in certain cases. The desired behavior is - // still to escape nested contexts if currently in one, but to - // insert a space if not (we don't expect mixed numbers in nested - // contexts). We should also limit to one consecutive space. spaceBehavesLikeTab: true, }); diff --git a/packages/math-input/src/components/input/scroll-into-view.ts b/packages/math-input/src/components/input/scroll-into-view.ts index 6cc303fbdc3..2cbed663741 100644 --- a/packages/math-input/src/components/input/scroll-into-view.ts +++ b/packages/math-input/src/components/input/scroll-into-view.ts @@ -4,9 +4,6 @@ * strong assumption that the keypad will be anchored to the bottom of the page * in calculating its height, as this method may be called before the keypad has * animated into view. - * - * TODO(charlie): Move this scroll logic out of our components and into a higher - * level in the component tree--perhaps even into khan/frontend, beyond Perseus. */ // HACK(charlie): This should be injected by webapp somehow. @@ -15,9 +12,6 @@ const toolbarHeightPx = 60; export const scrollIntoView = (containerNode, keypadNode) => { - // TODO(charlie): There's no need for us to be reading the keypad bounds - // here, since they're pre-determined by logic in the store. We should - // instead pass around an object that knows the bounds. const containerBounds = containerNode.getBoundingClientRect(); const containerBottomPx = containerBounds.bottom; const containerTopPx = containerBounds.top; diff --git a/packages/math-input/src/components/key-handlers/handle-backspace.ts b/packages/math-input/src/components/key-handlers/handle-backspace.ts index 458ecae959e..aeb1b398d6b 100644 --- a/packages/math-input/src/components/key-handlers/handle-backspace.ts +++ b/packages/math-input/src/components/key-handlers/handle-backspace.ts @@ -79,7 +79,6 @@ function handleBackspaceInRootIndex( // root's index, delete a character. mathField.keystroke("Backspace"); } else { - // TODO(kevinb) verify that we want this behavior after testing // Do nothing because we haven't completely deleted the // index of the radical. } diff --git a/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx b/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx index 2602b943681..6f67c727374 100644 --- a/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx +++ b/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx @@ -47,9 +47,7 @@ describe("keypad", () => { cursorContext={ context as (typeof CursorContext)[keyof typeof CursorContext] } - onAnalyticsEvent={async () => { - /* TODO: verify correct analytics event sent */ - }} + onAnalyticsEvent={async () => {}} />, ); diff --git a/packages/math-input/src/components/keypad/button-assets.tsx b/packages/math-input/src/components/keypad/button-assets.tsx index cd6352506c1..fb4a2abba95 100644 --- a/packages/math-input/src/components/keypad/button-assets.tsx +++ b/packages/math-input/src/components/keypad/button-assets.tsx @@ -401,7 +401,6 @@ export default function ButtonAsset({id}: Props): React.ReactNode { /> ); - // TODO: figure out what the difference is and what we need for what case "FRAC_INCLUSIVE": case "FRAC_EXCLUSIVE": case "FRAC": diff --git a/packages/perseus-core/src/data-schema.ts b/packages/perseus-core/src/data-schema.ts index 4733ba7dc0d..9051bf6912f 100644 --- a/packages/perseus-core/src/data-schema.ts +++ b/packages/perseus-core/src/data-schema.ts @@ -32,8 +32,6 @@ // be independent of everything else. import type {KeypadKey} from "./keypad"; -// TODO(FEI-4010): Remove `Perseus` prefix for all types here - export type Coord = [x: number, y: number]; export type Interval = [min: number, max: number]; export type Vector2 = Coord; // Same name as Mafs @@ -751,8 +749,6 @@ export type PerseusInteractiveGraphWidgetOptions = { */ rulerTicks?: number; // The X and Y coordinate ranges for the view of the graph. default: [[-10, 10], [-10, 10]] - // TODO(kevinb): Add a transform function to interactive-graph.jsx to - // rename `range` to `ranges` so that things are less confusing. range: GraphRange; // The type of graph graph: PerseusGraphType; diff --git a/packages/perseus-core/src/keypad.ts b/packages/perseus-core/src/keypad.ts index 19a44b5b21a..4fc831cfbce 100644 --- a/packages/perseus-core/src/keypad.ts +++ b/packages/perseus-core/src/keypad.ts @@ -29,7 +29,7 @@ export const KeypadKeys = [ "LOG", "LOG_N", "SIN", - "COS", // TODO(charlie): Add in additional Greek letters., + "COS", "TAN", "PI", "THETA", diff --git a/packages/perseus-core/src/traversal.ts b/packages/perseus-core/src/traversal.ts index fb9a81a5be8..7886eb11c31 100644 --- a/packages/perseus-core/src/traversal.ts +++ b/packages/perseus-core/src/traversal.ts @@ -2,8 +2,7 @@ * Traverses a {content, widgets, images} renderer props object, * such as `itemData.question` * - * This traversal is deep and handles some widget prop upgrades - * (TODO(aria): Handle minor prop upgrades :) ) + * This traversal is deep. * * This is the right way to traverse itemData. * diff --git a/packages/perseus-core/src/types.ts b/packages/perseus-core/src/types.ts index 7453b254cdc..2d2c80f9fb3 100644 --- a/packages/perseus-core/src/types.ts +++ b/packages/perseus-core/src/types.ts @@ -1,9 +1,6 @@ // Types that can be shared between Perseus packages // ideally without causing circular dependencies -// TODO: this should be typed -type State = any; - // Interfact currently only implemented by // ServerItemRenderer export interface RendererInterface { @@ -11,7 +8,7 @@ export interface RendererInterface { /** * @deprecated - do not use in new code. */ - getSerializedState(): State; + getSerializedState(): any; // TODO(LEMS-3185): remove serializedState blur(): void; focus(): boolean | null | undefined; diff --git a/packages/perseus-core/src/utils/get-decimal-separator.ts b/packages/perseus-core/src/utils/get-decimal-separator.ts index 899fa02e68f..b35015b3961 100644 --- a/packages/perseus-core/src/utils/get-decimal-separator.ts +++ b/packages/perseus-core/src/utils/get-decimal-separator.ts @@ -14,8 +14,8 @@ const getDecimalSeparator = (locale: string): string => { default: const numberWithDecimalSeparator = 1.1; - // TODO(FEI-3647): Update to use .formatToParts() once we no longer have to - // support Safari 12. + // TODO(FEI-3647): Update to use .formatToParts() since we no + // longer have to support Safari 12. const match = new Intl.NumberFormat(locale) .format(numberWithDecimalSeparator) // 0x661 is ARABIC-INDIC DIGIT ONE diff --git a/packages/perseus-core/src/utils/grapher-util.ts b/packages/perseus-core/src/utils/grapher-util.ts index 49afd57596f..de780b50139 100644 --- a/packages/perseus-core/src/utils/grapher-util.ts +++ b/packages/perseus-core/src/utils/grapher-util.ts @@ -20,8 +20,6 @@ export const MOVABLES = { SINUSOID: "SINUSOID", }; -// TODO(charlie): These really need to go into a utility file as they're being -// used by both interactive-graph and now grapher. function canonicalSineCoefficients(coeffs: any) { // For a curve of the form f(x) = a * Sin(b * x - c) + d, // this function ensures that a, b > 0, and c is its diff --git a/packages/perseus-core/src/widgets/expression/derive-extra-keys.ts b/packages/perseus-core/src/widgets/expression/derive-extra-keys.ts index 4c3f54f9a5f..c0692612486 100644 --- a/packages/perseus-core/src/widgets/expression/derive-extra-keys.ts +++ b/packages/perseus-core/src/widgets/expression/derive-extra-keys.ts @@ -60,8 +60,6 @@ function deriveExtraKeys( } } - // TODO(charlie): Alert the keypad as to which of these symbols should be - // treated as functions. const extraVariables = Object.keys( uniqueExtraVariables, ).sort() as ReadonlyArray; diff --git a/packages/perseus-editor/src/README.md b/packages/perseus-editor/src/README.md index 9a7a6789f52..cffa630b4fe 100644 --- a/packages/perseus-editor/src/README.md +++ b/packages/perseus-editor/src/README.md @@ -1,7 +1,4 @@ The Perseus editor, with all the widgets. -TODO(benkraft): Figure out and document how this relates to the other perseus -packages. - Code-Owner: Learning Platform Crowdin-Category: content.chrome diff --git a/packages/perseus-editor/src/__docs__/editor.stories.tsx b/packages/perseus-editor/src/__docs__/editor.stories.tsx index f970e524885..609eb842d47 100644 --- a/packages/perseus-editor/src/__docs__/editor.stories.tsx +++ b/packages/perseus-editor/src/__docs__/editor.stories.tsx @@ -72,7 +72,6 @@ export const DemoInteractiveGraph = (): React.ReactElement => { showWordCount={true} warnNoPrompt={false} warnNoWidgets={true} - // TODO(LEMS-2656): remove TS suppression onChange={ ((props: Partial) => { action("onChange")(props); diff --git a/packages/perseus-editor/src/article-editor.tsx b/packages/perseus-editor/src/article-editor.tsx index 4da7b8e3f6c..307bc83c951 100644 --- a/packages/perseus-editor/src/article-editor.tsx +++ b/packages/perseus-editor/src/article-editor.tsx @@ -491,8 +491,6 @@ export default class ArticleEditor extends React.Component { */ getSaveWarnings(): ReadonlyArray { if (this.props.mode !== "edit") { - // TODO(joshuan): We should be able to get save warnings in - // preview mode. throw new PerseusError( "Can only get save warnings in edit mode.", Errors.NotAllowed, diff --git a/packages/perseus-editor/src/components/checkbox.tsx b/packages/perseus-editor/src/components/checkbox.tsx index 6dfa5b4a18f..0d39f8f9490 100644 --- a/packages/perseus-editor/src/components/checkbox.tsx +++ b/packages/perseus-editor/src/components/checkbox.tsx @@ -168,7 +168,7 @@ const styles = StyleSheet.create({ bottom: -2, left: -2, borderRadius: borderRadius + 2, - backgroundColor: "lightblue", // TODO(kevinb) get real focus ring + backgroundColor: "lightblue", }, }, svg: { diff --git a/packages/perseus-editor/src/components/drag-target.tsx b/packages/perseus-editor/src/components/drag-target.tsx index 6319ccbcb41..956f1aea9cd 100644 --- a/packages/perseus-editor/src/components/drag-target.tsx +++ b/packages/perseus-editor/src/components/drag-target.tsx @@ -12,15 +12,6 @@ * the element will become partially transparent as a visual indicator that * it's a target. */ -// TODO(joel) - indicate before the hover is over the target that it's possible -// to drag into the target. This would (I think) require a high level handler - -// like on Perseus itself, waiting for onDragEnter, then passing down the -// event. Sounds like a pain. Possible workaround - create a div covering the -// entire page... -// -// Other extensions: -// * custom styles for global drag and dragOver -// * only respond to certain types of drags (only images for instance)! import * as React from "react"; diff --git a/packages/perseus-editor/src/components/dropdown-option.tsx b/packages/perseus-editor/src/components/dropdown-option.tsx index 79ea833ccee..aa4542ddbc4 100644 --- a/packages/perseus-editor/src/components/dropdown-option.tsx +++ b/packages/perseus-editor/src/components/dropdown-option.tsx @@ -148,10 +148,6 @@ class Option extends React.Component { } } -// TODO(kevinb): -// - keep the the option group within the page bounds -// - do we really want to dismiss this on scroll -// - how does the drop down interact with tooltips? what's the z-index order? class OptionGroup extends React.Component<{ // An event when an option is selected onSelected: (value: string) => void; diff --git a/packages/perseus-editor/src/components/form-wrapped-text-field.tsx b/packages/perseus-editor/src/components/form-wrapped-text-field.tsx index 28faa74024c..bc204f4b125 100644 --- a/packages/perseus-editor/src/components/form-wrapped-text-field.tsx +++ b/packages/perseus-editor/src/components/form-wrapped-text-field.tsx @@ -18,9 +18,6 @@ import type {components} from "@khanacademy/perseus"; type Props = { // This id is used to tie the input field to the label that describes it. - // TODO(diedra): Instead of passing in this id to connect the label and input - // field, we should pass in the label for the input field and place it - // inside the
. id?: string; testId?: string; type?: string; @@ -165,8 +162,6 @@ class FormWrappedTextField extends React.Component { ? styles.labelMediumInputBase : styles.inputBase; - // TODO(diedra): The label for the input field should be here inside of - // the form. return ( { } componentDidMount() { - // TODO(scottgrant): This is a hack to remove the deprecated call to + // NOTE(scottgrant): This is a hack to remove the deprecated call to // this.isMounted() but is still considered an anti-pattern. this._isMounted = true; @@ -168,8 +168,6 @@ class GraphSettings extends React.Component { return Changeable.change.apply(this, args); } - // TODO(aria): Make either a wrapper for standard events to work - // with this.change, or make these use some TextInput/NumberInput box changeRulerLabel(e) { this.change({rulerLabel: e.target.value}); } @@ -409,7 +407,6 @@ class GraphSettings extends React.Component { // true -> the settings are valid // a string -> the settings are invalid, and the explanation // is contained in the string - // TODO(aria): Refactor this to not be confusing const validationResult = this.validateGraphSettings( range, step, diff --git a/packages/perseus-editor/src/components/link.ts b/packages/perseus-editor/src/components/link.ts index 382b047bb38..068cf559416 100644 --- a/packages/perseus-editor/src/components/link.ts +++ b/packages/perseus-editor/src/components/link.ts @@ -9,9 +9,6 @@ import * as React from "react"; import type {CSSProperties} from "aphrodite"; -// TODO(diedra): This is terrible! If we don't have a href for a link, it should -// be a button instead. We just need to make sure we override the default browser -// button styling (backgroundColor, border, and padding). const DEFAULT_HREF = "javascript:void(0)"; type DefaultProps = { diff --git a/packages/perseus-editor/src/components/viewport-resizer.tsx b/packages/perseus-editor/src/components/viewport-resizer.tsx index d3356a98ba0..02ac2b36797 100644 --- a/packages/perseus-editor/src/components/viewport-resizer.tsx +++ b/packages/perseus-editor/src/components/viewport-resizer.tsx @@ -39,7 +39,6 @@ const ViewportResizer = (props: Props) => { ); - // TODO(david): Allow input of custom viewport sizes. return ( Viewport:{" "} diff --git a/packages/perseus-editor/src/components/widget-editor.tsx b/packages/perseus-editor/src/components/widget-editor.tsx index 1f4be4f4565..6bb517253b4 100644 --- a/packages/perseus-editor/src/components/widget-editor.tsx +++ b/packages/perseus-editor/src/components/widget-editor.tsx @@ -126,9 +126,6 @@ class WidgetEditor extends React.Component< }; serialize = () => { - // TODO(alex): Make this properly handle the case where we load json - // with a more recent widget version than this instance of Perseus - // knows how to handle. const widgetInfo = this.state.widgetInfo; return { type: widgetInfo.type, diff --git a/packages/perseus-editor/src/components/widget-select.tsx b/packages/perseus-editor/src/components/widget-select.tsx index 7deea38af5f..7a4c9592c77 100644 --- a/packages/perseus-editor/src/components/widget-select.tsx +++ b/packages/perseus-editor/src/components/widget-select.tsx @@ -14,7 +14,7 @@ class WidgetSelect extends React.Component { handleChange = (e: React.SyntheticEvent) => { const widgetType = e.currentTarget.value; if (widgetType === "") { - // TODO(alpert): Not sure if change will trigger here + // NOTE(alpert): Not sure if change will trigger here // but might as well be safe return; } diff --git a/packages/perseus-editor/src/editor-page.tsx b/packages/perseus-editor/src/editor-page.tsx index 4bf64c684c9..79ef6523b3e 100644 --- a/packages/perseus-editor/src/editor-page.tsx +++ b/packages/perseus-editor/src/editor-page.tsx @@ -115,7 +115,7 @@ class EditorPage extends React.Component { } componentDidMount() { - // TODO(scottgrant): This is a hack to remove the deprecated call to + // NOTE(scottgrant): This is a hack to remove the deprecated call to // this.isMounted() but is still considered an anti-pattern. this._isMounted = true; diff --git a/packages/perseus-editor/src/editor.tsx b/packages/perseus-editor/src/editor.tsx index 4e4f278f4cd..d326963ca15 100644 --- a/packages/perseus-editor/src/editor.tsx +++ b/packages/perseus-editor/src/editor.tsx @@ -357,7 +357,6 @@ class Editor extends React.Component { const imageUrl = dataTransfer.getData("URL"); if (imageUrl) { - // TODO(joel) - relocate when the image upload dialog lands const newContent = content + "\n\n![](" + imageUrl + ")"; // See componentDidUpdate() for how this flag is used this.lastUserValue = this.props.content; @@ -394,8 +393,6 @@ class Editor extends React.Component { } const sentinel = "\u2603 " + _.uniqueId("image_"); - // TODO(joel) - figure out how to temporarily include the image - // before the server returns. content += "\n\n![](" + sentinel + ")"; return {file: file, sentinel: sentinel}; @@ -506,7 +503,6 @@ class Editor extends React.Component { // before. Avoid name conflicts by renumbering pasted widgets so that // their numbers are always higher than the highest numbered widget of // their type. - // TODO(sam): Fix widget numbering in the widget editor titles const widgetJSON = localStorage.perseusLastCopiedWidgets; const lastCopiedText = localStorage.perseusLastCopiedText; @@ -516,10 +512,6 @@ class Editor extends React.Component { // Only intercept if we have widget data to paste and the user is // pasting something originally from Perseus. - // TODO(sam/aria/alex): Make it so that you can paste arbitrary text - // (e.g. from a text editor) instead of exactly what was copied, and - // let the widgetJSON match up with it. This would let you copy text - // into a buffer, perform complex operations on it, then paste it back. if (widgetJSON && lastCopiedText === textToBePasted) { e.preventDefault(); @@ -528,9 +520,6 @@ class Editor extends React.Component { this._safeWidgetNameMapping(widgetData); // Use safe widget name map to construct the new widget data - // TODO(aria/alex): Don't use `rWidgetSplit` or other piecemeal - // regexes directly; abstract this out so that we don't have to - // worry about potential edge cases. const safeWidgetData: Record = {}; for (const [key, data] of Object.entries(widgetData)) { safeWidgetData[safeWidgetMapping[key]] = data; @@ -914,13 +903,6 @@ class Editor extends React.Component { const type = match[2] as PerseusWidget["type"]; const selected = false; - // TODO(alpert): - // var selected = focused && selStart === selEnd && - // offset <= selStart && - // selStart < offset + text.length; - // if (selected) { - // selectedWidget = id; - // } const duplicate = id in widgets; @@ -938,19 +920,6 @@ class Editor extends React.Component { } } - // TODO(alpert): Move this to the content-change event handler - // _.each(_.keys(this.props.widgets), function(id) { - // if (!(id in widgets)) { - // // It's strange if these preloaded options stick around - // // since it's inconsistent with how things work if you - // // don't have the serialize/deserialize step in the - // // middle - // // TODO(alpert): Save options in a consistent manner so - // // that you can undo the deletion of a widget - // delete this.props.widgets[id]; - // } - // }, this); - this.widgetIds = _.keys(widgets); widgetsDropDown = ; diff --git a/packages/perseus-editor/src/hint-editor.tsx b/packages/perseus-editor/src/hint-editor.tsx index f46e5977604..92340aa0f0f 100644 --- a/packages/perseus-editor/src/hint-editor.tsx +++ b/packages/perseus-editor/src/hint-editor.tsx @@ -333,7 +333,6 @@ class CombinedHintsEditor extends React.Component { cb: () => unknown, silent: boolean, ) => { - // TODO(joel) - lens const hints = [...this.props.hints]; hints[i] = _.extend( {}, diff --git a/packages/perseus-editor/src/iframe-content-renderer.tsx b/packages/perseus-editor/src/iframe-content-renderer.tsx index 977f4cb142f..47f52456ae0 100644 --- a/packages/perseus-editor/src/iframe-content-renderer.tsx +++ b/packages/perseus-editor/src/iframe-content-renderer.tsx @@ -39,9 +39,6 @@ window.addEventListener("message", (event) => { updateIframeHeight[event.data.id](event.data.height); } else if (event.data.lintWarnings) { // This is a lint report being sent back from the linter. - // TODO: - // We'll want to display the number of warnings in the HUD. - // But for now, we just log it to the console Log.log("LINTER REPORT", { lintWarnings: JSON.stringify(event.data.lintWarnings), }); @@ -77,7 +74,7 @@ class IframeContentRenderer extends React.Component { iframeID: number; componentDidMount() { - // TODO(scottgrant): This is a hack to remove the deprecated call to + // NOTE(scottgrant): This is a hack to remove the deprecated call to // this.isMounted() but is still considered an anti-pattern. this._isMounted = true; diff --git a/packages/perseus-editor/src/widgets/categorizer-editor.tsx b/packages/perseus-editor/src/widgets/categorizer-editor.tsx index 82c9d5fac84..91b3c9fd2f7 100644 --- a/packages/perseus-editor/src/widgets/categorizer-editor.tsx +++ b/packages/perseus-editor/src/widgets/categorizer-editor.tsx @@ -85,7 +85,7 @@ class CategorizerEditor extends React.Component { // @ts-expect-error - TS2554 - Expected 3 arguments, but got 1. this.change({ items: items, - // TODO(eater): This truncates props.values so there + // NOTE(eater): This truncates props.values so there // are never more correct answers than items, // ensuring the widget is possible to answer // correctly. It doesn't necessarly keep each diff --git a/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index f6fa80b24ef..2d6f0776e86 100644 --- a/packages/perseus-editor/src/widgets/expression-editor.tsx +++ b/packages/perseus-editor/src/widgets/expression-editor.tsx @@ -131,11 +131,6 @@ class ExpressionEditor extends React.Component { } } }); - - // TODO(joel) - warn about: - // - unreachable answers (how??) - // - specific answers following unspecific answers - // - incorrect answers as the final form } return issues; diff --git a/packages/perseus-editor/src/widgets/input-number-editor.tsx b/packages/perseus-editor/src/widgets/input-number-editor.tsx index b6982829ce3..fd6376bbabb 100644 --- a/packages/perseus-editor/src/widgets/input-number-editor.tsx +++ b/packages/perseus-editor/src/widgets/input-number-editor.tsx @@ -153,8 +153,7 @@ class InputNumberEditor extends React.Component {