[Numeric Input] - Show "Format Options" Tooltip as List #1861

mark-fitzgerald · 2024-11-14T22:07:32Z

Summary:

Upon receiving focus, the Numeric Input field will show a tooltip indicating the format of the answer that it is expecting (assuming the input was configured with suggested formats). When multiple formats are indicated, they should be displayed as a list. Currently, the formats are indeed HTML list elements, but they don't display as list items (all formatting has been removed). This change fixes the visual formatting to display the items as a list, but only if there are two or more items. In the case of just one format example, the HTML list element is removed, and the content is displayed as regular text. Also, the first line in the tooltip is more of an introductory text than a format option, and therefore should not be included in the HTML list markup.

Issue: LEMS-2457

Test plan:

NOTE: Only the HTML modifications can be testing locally (aka Storybook). The visual modifications must be verified in a ZND (or locally in Webapp containerized) because the styling that interferes with the list styling does not exist in Storybook.

Storybook:

Launch Storybook
Navigate to Perseus Editor => Editor => Demo
Add a Numeric Input widget
Configure the widget to have just one format option
Move focus to the input field in the preview area
- The tooltip should NOT be a list
Configure the widget to have more than one format option
Move focus to the input field in the preview area
- The tooltip should be a list, with the first line of text NOT in the list

Webapp (locally or in a ZND):

Log into the app with a valid account for use in editing exercises
Navigate to a Numeric Input example
- Test Everything > Unit 2 > Lesson 2: Numeric input (ZND)
Edit the exercise
Configure the widget to have just one format option
Move focus to the input field in the preview area (make sure the "Desktop" preview is selected)
- The tooltip should NOT be a list
Configure the widget to have more than one format option
Move focus to the input field in the preview area
- The tooltip should be a list, with the first line of text NOT in the list

github-actions · 2024-11-14T22:17:13Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (5ae985c) and published it to npm. You
can install it using the tag PR1861.

Example:

yarn add @khanacademy/perseus@PR1861

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1861

github-actions · 2024-11-14T22:17:21Z

Size Change: +150 B (+0.01%)

Total Size: 1.29 MB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	423 kB	+150 B (+0.04%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.9 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.8 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-editor/dist/es/index.js`	697 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.55 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

Update snapshots to account for new tooltip layout.

…ded.

khan-actions-bot · 2024-11-18T22:47:19Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/many-keys-smash.md, packages/perseus/src/components/input-with-examples.tsx, packages/perseus/src/styles/perseus-renderer.less, packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap, packages/perseus/src/widgets/numeric-input/numeric-input.test.ts, packages/perseus/src/multi-items/__tests__/__snapshots__/multi-renderer.test.tsx.snap, packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap, packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set.test.ts.snap, packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap, packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

mark-fitzgerald · 2024-11-18T22:48:55Z

packages/perseus/src/widgets/numeric-input/numeric-input.test.ts

+
+        // Assert
+        expect(renderer).toHaveBeenAnsweredIncorrectly();
+    });


This isn't new. I just re-organized the tests to group them better. If the first test was "accept correct answer", I would think the next test would be "reject an incorrect answer". This also enabled me to group the snapshot tests together.

mark-fitzgerald · 2024-11-18T22:51:13Z

packages/perseus/src/styles/perseus-renderer.less

@@ -541,24 +541,20 @@
    }
 }

-// TODO(joel) - remove?
-.perseus-tooltip {


This style wasn't really used, other than to be a base later on in .perseus-formats-tooltip. So, I just moved this styling to where it is used.

mark-fitzgerald · 2024-11-18T22:51:47Z

packages/perseus/src/styles/perseus-renderer.less

    padding: 5px 10px;
    width: 240px;
 }

-// CSS is evil; let's overwrite some styles. T_T


No! CSS is not evil. Misunderstood, maybe. But definitely not evil.

mark-fitzgerald · 2024-11-18T22:53:32Z

packages/perseus/src/styles/perseus-renderer.less

-    margin: -20px 0 -16px 0;
-    > li {
-        list-style-type: none;
-    }


There's no evidence that this was ever needed. A list should (almost) always look like a list.

jeremywiebe

Nice!

jeremywiebe · 2024-11-19T00:24:03Z

packages/perseus/src/components/input-with-examples.tsx

+                      .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("**")


It looks like both remaining usages of <InputWithExamples /> provides an examples list where the first item is indeed a bold heading (here and here).

Perhaps we can simplify this once input-number is truly gone because at that point we'll know exactly what is always passed in.

That's the plan. We have follow-up tickets to refactor aspects of Numeric Input. Seeing as how InputWithExamples isn't used anywhere else, we are considering bringing this component into Numeric Input and simplifying the two code bases.

SonicScrewdriver

Seems straightforward - Thanks for all the cleanup work with the CSS!

catandthemachines

Looks pretty good. Just curious about the data-perseus attribute.

catandthemachines · 2024-11-21T23:55:03Z

packages/perseus/src/__tests__/__snapshots__/server-item-renderer.test.tsx.snap

+                          </div>
+                          <div
+                            class="paragraph"
+                            data-perseus-paragraph-index="1"


What is this data-perseus-paragraph-index attribute? Is it like our version of tabIndex?

Question has been posted to the team.

Adjust styling to show list bullets in tooltip.

dd182db

mark-fitzgerald self-assigned this Nov 14, 2024

Merge latest updates from main.

ee74650

Mark Fitzgerald added 7 commits November 14, 2024 16:33

Adjust how examples tooltip content is generated.

81ab282

Adjust styling to show list bullets (and fix margins).

2b1c4dd

Adjust styling to override styling that is too broad.

1b60da4

Update snapshots to account for new tooltip layout.

Merge branch 'main' into LEMS-2457-show-format-options-as-list

efc1e6a

Merge branch 'main' into LEMS-2457-show-format-options-as-list

886e67a

Only show tooltip content as a list if two or more examples are provi…

860e56b

…ded.

Add unit test to guard against accidental tooltip content changes.

7a6b4e7

mark-fitzgerald requested a review from SonicScrewdriver November 18, 2024 22:46

mark-fitzgerald marked this pull request as ready for review November 18, 2024 22:47

khan-actions-bot requested a review from a team November 18, 2024 22:47

mark-fitzgerald commented Nov 18, 2024

View reviewed changes

Merge branch 'main' into LEMS-2457-show-format-options-as-list

5ae985c

jeremywiebe approved these changes Nov 19, 2024

View reviewed changes

SonicScrewdriver approved these changes Nov 20, 2024

View reviewed changes

catandthemachines approved these changes Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Numeric Input] - Show "Format Options" Tooltip as List #1861

[Numeric Input] - Show "Format Options" Tooltip as List #1861

mark-fitzgerald commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

khan-actions-bot commented Nov 18, 2024

mark-fitzgerald Nov 18, 2024

mark-fitzgerald Nov 18, 2024

mark-fitzgerald Nov 18, 2024

mark-fitzgerald Nov 18, 2024

jeremywiebe left a comment

jeremywiebe Nov 19, 2024

mark-fitzgerald Nov 19, 2024

SonicScrewdriver left a comment

catandthemachines left a comment

catandthemachines Nov 21, 2024

mark-fitzgerald Nov 22, 2024

[Numeric Input] - Show "Format Options" Tooltip as List #1861

Are you sure you want to change the base?

[Numeric Input] - Show "Format Options" Tooltip as List #1861

Conversation

mark-fitzgerald commented Nov 14, 2024 • edited Loading

Summary:

Test plan:

Storybook:

Webapp (locally or in a ZND):

github-actions bot commented Nov 14, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Nov 14, 2024 • edited Loading

khan-actions-bot commented Nov 18, 2024

Gerald

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremywiebe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SonicScrewdriver left a comment

Choose a reason for hiding this comment

catandthemachines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mark-fitzgerald commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading