[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 (07ac191) 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: +137 B (+0.01%)

Total Size: 1.29 MB

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

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39 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.9 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.7 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.5 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.

@Myranae

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Minor Changes - [#1901](#1901) [`051c50cf7`](051c50c) Thanks [@Myranae](https://github.com/Myranae)! - Introduces a validation function for the number line widget (extracted from the scoring function). - [#1936](#1936) [`d05272661`](d052726) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Changes the PerseusWidgetsMap to be extensible so that widgets can be registered outside of Perseus and still have full type safety. - [#1832](#1832) [`e3062b3c8`](e3062b3) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Numeric Input] - Update the UI to match Expression widget ### Patch Changes - [#1937](#1937) [`3cdabf1a3`](3cdabf1) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - TypeScript fixes - [#1948](#1948) [`e21a3a39b`](e21a3a3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Refactor internally used object mapping utilities to use ES6 exports - [#1938](#1938) [`5e8d8468b`](5e8d846) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Type fixes - [#1942](#1942) [`1d2b4e7bf`](1d2b4e7) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Ensure that zoomed-in images retain alt text - [#1861](#1861) [`763a4ba38`](763a4ba) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Numeric Input] - Show format options as a list - [#1947](#1947) [`b8926e38a`](b8926e3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Minor refactoring of ServerItemRenderer's componentDidUpdate to reduce duplication - [#1946](#1946) [`f35512786`](f355127) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Refactor scoring for `group` widget to follow the same pattern as all other widgets - [#1891](#1891) [`ef819ea95`](ef819ea) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Numeric Input] - Associate format options tooltip content with input field for assistive technologies - [#1945](#1945) [`e69ca3146`](e69ca31) Thanks [@nishasy](https://github.com/nishasy)! - Add global styles to reflect prod styling - [#1923](#1923) [`be8c06c75`](be8c06c) Thanks [@benchristel](https://github.com/benchristel)! - Internal: convert backgroundImage dimensions to numbers during parsing. - [#1934](#1934) [`129adebef`](129adeb) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Improve comments on some Perseus types - [#1924](#1924) [`2d89ef87d`](2d89ef8) Thanks [@benchristel](https://github.com/benchristel)! - Internal: add and pass regression tests for PerseusItem parser's handling of legacy data ## @khanacademy/[email protected] ### Patch Changes - [#1938](#1938) [`5e8d8468b`](5e8d846) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Type fixes - [#1937](#1937) [`3cdabf1a3`](3cdabf1) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Ensure links opening to style guide (Google Docs) set `rel="noreferrer"` - [#1946](#1946) [`f35512786`](f355127) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove debugging call in GraphSettings component - Updated dependencies \[[`3cdabf1a3`](3cdabf1), [`e21a3a39b`](e21a3a3), [`5e8d8468b`](5e8d846), [`1d2b4e7bf`](1d2b4e7), [`763a4ba38`](763a4ba), [`b8926e38a`](b8926e3), [`f35512786`](f355127), [`ef819ea95`](ef819ea), [`e69ca3146`](e69ca31), [`be8c06c75`](be8c06c), [`051c50cf7`](051c50c), [`129adebef`](129adeb), [`d05272661`](d052726), [`2d89ef87d`](2d89ef8), [`e3062b3c8`](e3062b3)]: - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ⏭️ Publish npm snapshot, ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1944

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

Mark Fitzgerald added 3 commits December 3, 2024 09:39

Merge latest from main.

b4f21e4

Update snapshots after merging in main.

3bf13ec

Update snapshots after merging in main.

07ac191

mark-fitzgerald merged commit 763a4ba into main Dec 3, 2024
9 checks passed

mark-fitzgerald deleted the LEMS-2457-show-format-options-as-list branch December 3, 2024 21:03

khan-actions-bot mentioned this pull request Dec 3, 2024

Version Packages #1944

Merged

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 •

edited

Loading

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

[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 • edited Loading

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

khan-actions-bot commented Nov 18, 2024 •

edited

Loading