[Radio] Refactor multiple choice indicator (logic and UI updates) #2687

mark-fitzgerald · 2025-07-08T00:14:35Z

Summary:

This PR refactors the logic, HTML, and CSS for the choice icon, now called choice-indicator. It replaces the code from choice-icon.* and focus-ring.tsx.

To account for product theming and dark mode styling, CSS variables from Wonder Blocks is used throughout. CSS tokens are used instead of Wonder Blocks components since the multiple choice indicator use case doesn't fit the intended use cases of the components available.

Issue: LEMS-3173

Test plan:

Launch Storybook locally
Navigate to Perseus => Widgets => Radio => Indicator => All Settings
Visually, the different states match the new specs.
Upon inspection, notice that CSS variables are used in appropriate places.

…ton with basic styling. Initial Storybook file for testing.

Add icon for review mode (aka correctness). Prevent button execution when in review mode. Adjust styling for various states (i.e. hover, review mode). Add stories. Add unit tests.

…dard HTML button instead of WB Button. Style button to use WB tokens (CSS variables). Update unit tests accordingly.

Fix handleClick function to send correct state value for isChecked.

…tor multiple choice indicator (logic and UI updates)

Update snapshots for multiple choice widget to account for proper handling of CSS Modules imports.

github-actions · 2025-07-08T00:16:04Z

Size Change: 0 B

Total Size: 481 kB

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.7 kB
`packages/keypad-context/dist/es/index.js`	1 kB
`packages/kmath/dist/es/index.js`	5.98 kB
`packages/math-input/dist/es/index.js`	98.6 kB
`packages/math-input/dist/es/strings.js`	1.61 kB
`packages/perseus-core/dist/es/index.js`	21.2 kB
`packages/perseus-editor/dist/es/index.js`	91.3 kB
`packages/perseus-linter/dist/es/index.js`	7.14 kB
`packages/perseus-score/dist/es/index.js`	9.25 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/index.js`	208 kB
`packages/perseus/dist/es/strings.js`	7.56 kB
`packages/pure-markdown/dist/es/index.js`	1.22 kB
`packages/simple-markdown/dist/es/index.js`	6.72 kB

_{compressed-size-action}

github-actions · 2025-07-08T00:16:39Z

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2687

If you are working in Khan Academy's frontend, you can run the below command.

./tools/bump_perseus_version.ts -t PR2687

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR2687

Change name to "choice-indicator" (removing "multiple").

config/test/test.config.js

mark-fitzgerald · 2025-07-08T00:58:12Z

config/test/test.config.js

@@ -75,7 +76,7 @@ module.exports = {
        ...pkgMap,
        ...vendorMap,
        // Load a .js file with no exports whenever a .css file is requested.
-        "\\.(css)$": "<rootDir>/config/test/style-mock.js",
+        "(?<!\\.module)\\.css$": "<rootDir>/config/test/style-mock.js",


I adjusted this entry to only be applied on regular CSS files, but to not apply to CSS Module files (otherwise, it would conflict with the transform instructions on line 57).

mark-fitzgerald · 2025-07-08T01:01:49Z

packages/perseus/src/widgets/radio/__tests__/__snapshots__/multiple-choice.test.ts.snap

Snapshots that used components that referenced CSS Module files were updated due to the Jest configuration change.

ivyolamit

@mark-fitzgerald here's my first round of review comments. I'll do a second round shortly after my meetings 🙏🏼

packages/perseus/src/widgets/radio/__stories__/indicator.stories.tsx

packages/perseus/src/widgets/radio/choice-indicator.module.css

ivyolamit · 2025-07-08T15:52:49Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    border: solid 0.2rem var(--wb-semanticColor-core-border-neutral-strong);
+    color: var(--wb-semanticColor-core-foreground-neutral-default);
+    cursor: pointer;
+    font-family: var(--wb-font-family-sans);


is this something we want to define here? and not in global?

Preferably, this would be set somewhere higher up. I'm not sure that we can rely upon a proper ancestor setting, though. Probably something that I need to check with the team.

what do you think of adding todo comment here for now so we won't forget that it would be preferable to have this on higher level instead of here?

It makes sense to declare this higher up, but it is probably easier to do a full css pass as we integrate the new design, and after we have the individual components all hooked up. We have a lot of very specific css rules floating around in Perseus that it might be simpler to do it holistically

packages/perseus/src/widgets/radio/choice-indicator.module.css

ivyolamit · 2025-07-08T15:55:37Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    line-height: var(--wb-font-heading-lineHeight-small);
+    outline: 0 solid var(--wb-semanticColor-core-background-instructive-default);
+    outline-offset: 0;
+    padding: 0;


Suggested change

padding: 0;

padding: var(--wb-sizing-size_0);

packages/perseus/src/widgets/radio/choice-indicator.module.css

ivyolamit · 2025-07-08T15:58:47Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    background-color: var(
+        --wb-semanticColor-core-background-instructive-default
+    );
+    border-width: 0;


same comment with 0 above

ivyolamit · 2025-07-08T15:59:15Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+
+/* icon inside the button when showing correctness */
+.base [role="img"] {
+    margin-inline-end: 0.4rem;


Suggested change

margin-inline-end: 0.4rem;

margin-inline-end: var(--wb-sizing-size_040);

Changing to 1/3 of an em. The space between the icon and the choice letter should scale visually with the font-size. Even though the checkbox is an icon, it is acting like text (i.e. similar to unicode icons). The space between the icon and the letter should be proportional to their size. By setting the margin to an em measurement, we don't have to worry about adjusting this space if we change the font size of the letter, and it will scale with any user-based font-size changes.

packages/perseus/src/widgets/radio/choice-indicator.module.css

packages/perseus/src/widgets/radio/choice-indicator.new.tsx

…3137-modernize-option-indicator

…more from Wonder Blocks (mostly for sizing). Move icon sizing to CSS file. Adjust styling to account for container context (i.e. flex shrinking, additional hover detection). Add reference handling so that container can "click" the indicator without having to require the user to click the indicator button itself. Make sure that only one click event is registered when clicking the container. Update story and unit test files, accordingly.

packages/perseus/src/widgets/radio/__tests__/multiple-choice-indicator.test.tsx

ivyolamit · 2025-07-11T22:58:20Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    flex-shrink: 0;
+    font-family: var(--wb-font-family-sans);
+    font-size: var(--wb-font-heading-size-small);
+    height: var(--wb-c-button-root-sizing-height-small);


i can't seem to find this in the docs, is this something available for us to use?

It's not in the docs. In fact, they cautioned against using it. However, they also didn't provide an alternative to our needs. I suppose setting our own size here would be more appropriate, since the size of this button isn't tied to anything in Wonder Blocks. I'll update accordingly.

@mark-fitzgerald just saw this in

node_modules/.pnpm/@khanacademy+wonder-blocks-button@10.2.11_@phosphor-icons+core@2.0.2_aphrodite@1.2.5_re_f6b95c8f1f4f35b85908aaae53936269/node_modules/@khanacademy/wonder-blocks-button/dist/css/vars.css

has --wb-c-button-root-sizing-height-small: var(--wb-sizing-size_320); ?

if we can't use --wb-sizing-size_320 please add a comment on it so no-one will copy it

SonicScrewdriver

This looks great to me! Thanks for the amazing comments and explanations for the unique CSS applications

config/test/test.config.js

packages/perseus/src/widgets/radio/choice-indicator.module.css

SonicScrewdriver · 2025-07-11T22:49:37Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    border: solid 0.2rem var(--wb-semanticColor-core-border-neutral-strong);
+    color: var(--wb-semanticColor-core-foreground-neutral-default);
+    cursor: pointer;
+    font-family: var(--wb-font-family-sans);


It makes sense to declare this higher up, but it is probably easier to do a full css pass as we integrate the new design, and after we have the individual components all hooked up. We have a lot of very specific css rules floating around in Perseus that it might be simpler to do it holistically

SonicScrewdriver · 2025-07-11T22:54:54Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+.is-correct[aria-pressed="true"] {
+    background-color: var(--wb-semanticColor-core-background-success-strong);
+    color: var(
+        --wb-semanticColor-action-primary-progressive-default-foreground


One thing that I'm not the biggest fan of is how long these tokens can get.

SonicScrewdriver · 2025-07-11T22:55:16Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+    );
+    width: calc(
+        1.5em + var(--wb-c-button-root-sizing-height-small)
+    ); /* adding 1.5em accounts for the checkmark icon */


Greatly appreciate the explanation here!

ivyolamit · 2025-07-11T23:02:33Z

packages/perseus/src/widgets/radio/choice-indicator.module.css

+       If we change the font size of the letter, we want the icon to scale proportionally.
+       The space between the icon and the letter should also be proportional to their size.
+       By setting the width, height, and margin to an em unit,
+           we don't have to worry about adjusting these if we change the font size of the letter,


this is just a nitpick, please properly aligning any multi-line comments

ivyolamit

@mark-fitzgerald overall PR looks good. I have few comments, but i'll leave it up to you.
And also i think one of the file was missed in renaming multiple-choice-indicator.test.tsx --> choice-indicator.test.tsx

Change file name to remove `multiple-`.

mark-fitzgerald added 6 commits June 17, 2025 17:44

[LEMS-3137-modernize-option-indicator] Initial setup of indicator but…

0c4dc63

…ton with basic styling. Initial Storybook file for testing.

[LEMS-3137-modernize-option-indicator] Add aria-pressed state for a11y.

0e40929

Add icon for review mode (aka correctness). Prevent button execution when in review mode. Adjust styling for various states (i.e. hover, review mode). Add stories. Add unit tests.

[LEMS-3137-modernize-option-indicator] Refactor indicator to use stan…

e366dad

…dard HTML button instead of WB Button. Style button to use WB tokens (CSS variables). Update unit tests accordingly.

[LEMS-3137-modernize-option-indicator] Finish unit tests.

36d16cb

Fix handleClick function to send correct state value for isChecked.

[LEMS-3137-modernize-option-indicator] docs(changeset): [Radio] Refac…

8b35ce5

…tor multiple choice indicator (logic and UI updates)

[LEMS-3137-modernize-option-indicator] Update with latest from main.

f45fc59

Update snapshots for multiple choice widget to account for proper handling of CSS Modules imports.

mark-fitzgerald requested review from ivyolamit, SonicScrewdriver, anakaren-rojas and nishasy as code owners July 8, 2025 00:14

mark-fitzgerald self-assigned this Jul 8, 2025

mark-fitzgerald added the olc-5.0.d08c8 label Jul 8, 2025

[LEMS-3137-modernize-option-indicator] Adjust CSS formatting (Prettier).

26152aa

Change name to "choice-indicator" (removing "multiple").

mark-fitzgerald commented Jul 8, 2025

View reviewed changes

config/test/test.config.js Show resolved Hide resolved

mark-fitzgerald commented Jul 8, 2025

View reviewed changes

ivyolamit reviewed Jul 8, 2025

View reviewed changes

packages/perseus/src/widgets/radio/choice-indicator.new.tsx Show resolved Hide resolved

ivyolamit reviewed Jul 8, 2025

View reviewed changes

packages/perseus/src/widgets/radio/choice-indicator.new.tsx Outdated Show resolved Hide resolved

ivyolamit added the project: new radio widget Pull requests that is related to the Radio Widget project that will build the new radio widget label Jul 9, 2025

mark-fitzgerald added 3 commits July 10, 2025 16:55

[LEMS-3137-modernize-option-indicator] Merge branch 'main' into LEMS-…

8c50636

…3137-modernize-option-indicator

[LEMS-3137-modernize-option-indicator] Prettier

67cde7f

ivyolamit reviewed Jul 11, 2025

View reviewed changes

packages/perseus/src/widgets/radio/__tests__/multiple-choice-indicator.test.tsx Outdated Show resolved Hide resolved

ivyolamit reviewed Jul 11, 2025

View reviewed changes

SonicScrewdriver approved these changes Jul 11, 2025

View reviewed changes

ivyolamit reviewed Jul 11, 2025

View reviewed changes

ivyolamit approved these changes Jul 11, 2025

View reviewed changes

[LEMS-3137-modernize-option-indicator] Adjust for PR comments.

387c5c2

Change file name to remove `multiple-`.

mark-fitzgerald merged commit 611da42 into main Jul 11, 2025
8 checks passed

mark-fitzgerald deleted the LEMS-3137-modernize-option-indicator branch July 11, 2025 23:37

khan-actions-bot mentioned this pull request Jul 11, 2025

Version Packages #2700

Open

	margin-inline-end: 0.4rem;
	margin-inline-end: var(--wb-sizing-size_040);

[Radio] Refactor multiple choice indicator (logic and UI updates) #2687

[Radio] Refactor multiple choice indicator (logic and UI updates) #2687

Conversation

mark-fitzgerald commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test plan:

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

npm Snapshot: Published

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivyolamit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivyolamit Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivyolamit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mark-fitzgerald commented Jul 8, 2025 •

edited

Loading

github-actions bot commented Jul 8, 2025 •

edited

Loading

github-actions bot commented Jul 8, 2025 •

edited

Loading

ivyolamit Jul 11, 2025 •

edited

Loading