From dd182db13076c77afa3458897db51e6e5033e0e3 Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 14 Nov 2024 14:05:25 -0800 Subject: [PATCH 1/6] Adjust styling to show list bullets in tooltip. --- .changeset/many-keys-smash.md | 5 +++++ packages/perseus/src/styles/perseus-renderer.less | 10 +++++++++- .../src/widgets/numeric-input/numeric-input.tsx | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 .changeset/many-keys-smash.md diff --git a/.changeset/many-keys-smash.md b/.changeset/many-keys-smash.md new file mode 100644 index 0000000000..3435bb94bd --- /dev/null +++ b/.changeset/many-keys-smash.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +[Numeric Input] - Show format options as a list diff --git a/packages/perseus/src/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index 388c1a317c..0a0ac2140d 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -553,7 +553,7 @@ .perseus-tooltip; color: #777; } -.framework-perseus .perseus-formats-tooltip .paragraph > ul { +.framework-perseus :not(.perseus-numeric-input /* Not in my widget, buddy! */) .perseus-formats-tooltip .paragraph > ul { padding: 0; margin: -20px 0 -16px 0; > li { @@ -561,6 +561,14 @@ } } +.perseus-numeric-input .perseus-formats-tooltip ul { + padding-inline-start: 20px; + > li:first-of-type { + list-style-type: none; + margin-inline-start: -20px; + } +} + .box-shadow(@shadow: 0 1px 3px rgba(0,0,0,0.25)) { box-shadow: @shadow; } diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx index fc4fbb0e70..771cd00f98 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx @@ -249,7 +249,7 @@ export class NumericInput }); return ( -
+
(this.inputRef = ref)} value={this.props.currentValue} From 81ab282cc7db6850341fb42c4955663f4b57ae5b Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 14 Nov 2024 16:33:46 -0800 Subject: [PATCH 2/6] Adjust how examples tooltip content is generated. --- .../src/components/input-with-examples.tsx | 12 ++++++-- .../widgets/numeric-input/numeric-input.tsx | 28 +++++++++---------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/perseus/src/components/input-with-examples.tsx b/packages/perseus/src/components/input-with-examples.tsx index da253dfec3..e40a9a3575 100644 --- a/packages/perseus/src/components/input-with-examples.tsx +++ b/packages/perseus/src/components/input-with-examples.tsx @@ -146,9 +146,15 @@ class InputWithExamples extends React.Component { render(): React.ReactNode { const input = this._renderInput(); - const examplesContent = _.map(this.props.examples, (example) => { - return "- " + example; - }).join("\n"); + const examplesContent = this.props.examples + .map((example, index) => { + // If the first example is bold, then it is most likely a heading/leading text. + // So, it shouldn't be part of the list. + return index === 0 && example.startsWith("**") + ? `${example}\n` + : `- ${example}`; + }) + .join("\n"); const showExamples = this.props.shouldShowExamples && this.state.showExamples; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx index 6926b053c0..a162b2e564 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx @@ -249,21 +249,19 @@ export class NumericInput }); return ( -
- (this.inputRef = ref)} - value={this.props.currentValue} - onChange={this.handleChange} - labelText={labelText} - examples={this.examples()} - shouldShowExamples={this.shouldShowExamples()} - onFocus={this._handleFocus} - onBlur={this._handleBlur} - id={this.props.widgetId} - disabled={this.props.apiOptions.readOnly} - style={styles.input} - /> -
+ (this.inputRef = ref)} + value={this.props.currentValue} + onChange={this.handleChange} + labelText={labelText} + examples={this.examples()} + shouldShowExamples={this.shouldShowExamples()} + onFocus={this._handleFocus} + onBlur={this._handleBlur} + id={this.props.widgetId} + disabled={this.props.apiOptions.readOnly} + style={styles.input} + /> ); } } From 2b1c4dd8b5dd6100bc3bbd2dda1a665c516c463e Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 14 Nov 2024 17:08:37 -0800 Subject: [PATCH 3/6] Adjust styling to show list bullets (and fix margins). --- .../perseus/src/styles/perseus-renderer.less | 26 ++++--------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/perseus/src/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index 0a0ac2140d..880701d108 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -541,32 +541,16 @@ } } -// TODO(joel) - remove? -.perseus-tooltip { +.perseus-formats-tooltip { background: #fff; + color: #777; padding: 5px 10px; width: 240px; } -// CSS is evil; let's overwrite some styles. T_T -.perseus-formats-tooltip { - .perseus-tooltip; - color: #777; -} -.framework-perseus :not(.perseus-numeric-input /* Not in my widget, buddy! */) .perseus-formats-tooltip .paragraph > ul { - padding: 0; - margin: -20px 0 -16px 0; - > li { - list-style-type: none; - } -} - -.perseus-numeric-input .perseus-formats-tooltip ul { - padding-inline-start: 20px; - > li:first-of-type { - list-style-type: none; - margin-inline-start: -20px; - } +.perseus-formats-tooltip .paragraph, +.perseus-formats-tooltip ul { + margin: 0; } .box-shadow(@shadow: 0 1px 3px rgba(0,0,0,0.25)) { From 1b60da4121ac1c016daec05841d7b1814ddc10ae Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Fri, 15 Nov 2024 12:06:05 -0800 Subject: [PATCH 4/6] Adjust styling to override styling that is too broad. Update snapshots to account for new tooltip layout. --- .../server-item-renderer.test.tsx.snap | 18 +++++-- .../multi-renderer.test.tsx.snap | 18 +++++-- .../perseus/src/styles/perseus-renderer.less | 8 ++- .../graded-group-set-jipt.test.ts.snap | 54 +++++++++++++------ .../graded-group-set.test.ts.snap | 18 +++++-- .../group/__snapshots__/group.test.tsx.snap | 36 +++++++++---- .../__snapshots__/numeric-input.test.ts.snap | 36 +++++++++---- 7 files changed, 136 insertions(+), 52 deletions(-) diff --git a/packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap b/packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap index da69cf654c..c681f068b5 100644 --- a/packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap +++ b/packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap @@ -82,13 +82,21 @@ exports[`server item renderer should snapshot: initial render 1`] = `
+
+ + Your answer should be + + +
+
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like +
    + + Your answer should be + + +
    +
+
    -
  • - - Your answer should be - -
  • an integer, like Date: Mon, 18 Nov 2024 11:23:08 -0800 Subject: [PATCH 5/6] Only show tooltip content as a list if two or more examples are provided. --- .../src/components/input-with-examples.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/perseus/src/components/input-with-examples.tsx b/packages/perseus/src/components/input-with-examples.tsx index e40a9a3575..6208a55d83 100644 --- a/packages/perseus/src/components/input-with-examples.tsx +++ b/packages/perseus/src/components/input-with-examples.tsx @@ -146,15 +146,18 @@ class InputWithExamples extends React.Component { render(): React.ReactNode { const input = this._renderInput(); - const examplesContent = this.props.examples - .map((example, index) => { - // If the first example is bold, then it is most likely a heading/leading text. - // So, it shouldn't be part of the list. - return index === 0 && example.startsWith("**") - ? `${example}\n` - : `- ${example}`; - }) - .join("\n"); + const examplesContent = + this.props.examples.length <= 2 + ? this.props.examples.join(" ") // A single item (with or without leading text) is not a "list" + : this.props.examples // 2 or more items should display as a list + .map((example, index) => { + // If the first example is bold, then it is most likely a heading/leading text. + // So, it shouldn't be part of the list. + return index === 0 && example.startsWith("**") + ? `${example}\n` + : `- ${example}`; + }) + .join("\n"); const showExamples = this.props.shouldShowExamples && this.state.showExamples; From 7a6b4e7f7f50bfc03fa8b8875805236b9a6c577b Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Mon, 18 Nov 2024 14:45:34 -0800 Subject: [PATCH 6/6] Add unit test to guard against accidental tooltip content changes. --- .../__snapshots__/numeric-input.test.ts.snap | 262 ++++++++++++++++++ .../numeric-input/numeric-input.test.ts | 42 ++- 2 files changed, 297 insertions(+), 7 deletions(-) diff --git a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap index dd1659dad6..074f7f7888 100644 --- a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap +++ b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap @@ -482,3 +482,265 @@ exports[`numeric-input widget Should render predictably: first render 1`] = `
`; + +exports[`numeric-input widget Should render tooltip as list when multiple format options are given: render with format list tooltip 1`] = ` +
+
+
+
+ + + + 5008 \\div 4 = + + + + +
+ +
+ +
+
+
+
+
+
+
+
+
+
+
+
+ + Your answer should be + + +
+
+
+
    +
  • + a + + simplified proper + + fraction, like + + + + 3/5 + + + +
    • +
  • + a + + simplified improper + + fraction, like + + + + 7/4 + + + +
    • +
  • + a mixed number, like + + + + 1\\ 3/4 + + + +
    • +
+
+
+
+
+
+
+ +
+ +
+
+
+
+`; + +exports[`numeric-input widget Should render tooltip when format option is given: render with format tooltip 1`] = ` +
+
+
+
+ + + + 5008 \\div 4 = + + + + +
+ +
+ +
+
+
+
+
+
+
+
+
+
+
+
+ + Your answer should be + + a + + simplified proper + + fraction, like + + + + 3/5 + + + +
+
+
+
+
+
+
+ +
+ +
+
+
+
+`; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts index 2740d841d3..7a36eaa481 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts @@ -47,6 +47,20 @@ describe("numeric-input widget", () => { expect(renderer).toHaveBeenAnsweredCorrectly(); }); + it("should reject an incorrect answer", async () => { + // Arrange + const {renderer} = renderQuestion(question); + + // Act + await userEvent.type( + screen.getByRole("textbox", {hidden: true}), + incorrect, + ); + + // Assert + expect(renderer).toHaveBeenAnsweredIncorrectly(); + }); + it("Should render predictably", async () => { // Arrange const {container} = renderQuestion(question); @@ -62,18 +76,32 @@ describe("numeric-input widget", () => { expect(container).toMatchSnapshot("after interaction"); }); - it("should reject an incorrect answer", async () => { + it("Should render tooltip when format option is given", async () => { // Arrange - const {renderer} = renderQuestion(question); + const questionWithFormatOptions = JSON.parse(JSON.stringify(question1)); + questionWithFormatOptions.widgets[ + "numeric-input 1" + ].options.answers[0].answerForms = ["proper"]; // Act - await userEvent.type( - screen.getByRole("textbox", {hidden: true}), - incorrect, - ); + const {container} = renderQuestion(questionWithFormatOptions); // Assert - expect(renderer).toHaveBeenAnsweredIncorrectly(); + expect(container).toMatchSnapshot("render with format tooltip"); + }); + + it("Should render tooltip as list when multiple format options are given", async () => { + // Arrange + const questionWithFormatOptions = JSON.parse(JSON.stringify(question1)); + questionWithFormatOptions.widgets[ + "numeric-input 1" + ].options.answers[0].answerForms = ["proper", "improper", "mixed"]; + + // Act + const {container} = renderQuestion(questionWithFormatOptions); + + // Assert + expect(container).toMatchSnapshot("render with format list tooltip"); }); });