Skip to content

Commit e539e6d

Browse files
[Radio] Refactor multiple choice option (logic and UI updates) (#2697)
## Summary: This PR refactors the logic, HTML, and CSS for the choice option. This component was simplified to just handle the display of the option content and its styling. Accessibility features will be added after the component is refactored so that it can be applied in a holistic manner across the various files. Issue: LEMS-3168 ## Test plan: 1. Launch Storybook locally 1. Navigate to Perseus => Widgets => Radio => Widget Internal Components => [Single Select](http://localhost:6006/?path=/story/perseus-widgets-radio-widget-internal-components-choice--single-select) 1. Visually, the different states match the new specs. - Note that clicking anywhere in the option will toggle the indicator (for the options that are not in review mode). Author: mark-fitzgerald Reviewers: ivyolamit, SonicScrewdriver, mark-fitzgerald, anakaren-rojas, nishasy Required Reviewers: Approved By: SonicScrewdriver, ivyolamit Checks: ✅ 8 checks were successful Pull Request URL: #2697
1 parent d0d03f8 commit e539e6d

File tree

6 files changed

+309
-5
lines changed

6 files changed

+309
-5
lines changed

.changeset/fifty-planes-drum.md

Lines changed: 0 additions & 5 deletions
This file was deleted.

.changeset/fluffy-phones-kick.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/perseus": minor
3+
---
4+
5+
[Radio] Refactor multiple choice option (logic and UI updates)
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import * as React from "react";
2+
3+
import Choice from "../multiple-choice-option.new";
4+
5+
const Container = (props: {children: React.ReactNode}): React.ReactElement => {
6+
return (
7+
<ul
8+
style={{
9+
margin: "10px",
10+
width: "672px",
11+
}}
12+
>
13+
{props.children}
14+
</ul>
15+
);
16+
};
17+
18+
export default {
19+
title: "Perseus/Widgets/Radio/Widget Internal Components/Choice",
20+
};
21+
22+
export const SingleSelect = (): React.ReactElement => {
23+
return (
24+
<Container>
25+
<Choice
26+
checked={false}
27+
indicatorContent="A"
28+
isMultiSelect={false}
29+
updateChecked={() => {}}
30+
>
31+
USS Sarajevo - NCC-38529
32+
</Choice>
33+
<Choice
34+
checked={false}
35+
indicatorContent="B"
36+
isMultiSelect={false}
37+
updateChecked={() => {}}
38+
>
39+
USS Yangtzee Kiang - NCC-72453 - Hijacked by Bajoran terrorist
40+
Tahna Los. Became the first DS9 runabout to be destroyed when it
41+
crashed on a moon in the Gamma Quadrant.
42+
</Choice>
43+
<Choice
44+
checked={false}
45+
indicatorContent="C"
46+
isMultiSelect={false}
47+
updateChecked={() => {}}
48+
>
49+
<img
50+
alt="triangle"
51+
src="https://cdn.kastatic.org/ka-content-images/9cb2cf618c16501d01abf97036deb355d9393949.png"
52+
/>
53+
</Choice>
54+
<Choice
55+
checked={true}
56+
indicatorContent="D"
57+
isMultiSelect={false}
58+
showCorrectness="wrong"
59+
updateChecked={() => {}}
60+
>
61+
ISS Enterprise - NCC-1701 (Mirror Universe)
62+
</Choice>
63+
<Choice
64+
checked={false}
65+
indicatorContent="E"
66+
isMultiSelect={false}
67+
updateChecked={() => {}}
68+
>
69+
USS Yamaguchi - NCC-26510
70+
</Choice>
71+
<Choice
72+
checked={true}
73+
indicatorContent="F"
74+
isMultiSelect={false}
75+
showCorrectness="correct"
76+
updateChecked={() => {}}
77+
>
78+
USS Enterprise - NCC-1701
79+
</Choice>
80+
</Container>
81+
);
82+
};
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import {describe, it} from "@jest/globals";
2+
import {act, render, screen} from "@testing-library/react";
3+
import * as React from "react";
4+
5+
import * as IndicatorComponent from "../choice-indicator.new";
6+
import Choice from "../multiple-choice-option.new";
7+
8+
describe("Multiple choice option", () => {
9+
it.each`
10+
isMultiSelect | isOrIsNot | shape
11+
${false} | ${"is NOT"} | ${"circle"}
12+
${true} | ${"is"} | ${"square"}
13+
`(
14+
"renders with a $shape indicator when choice $isOrIsNot multi-select",
15+
(args) => {
16+
const {isMultiSelect, shape} = args as {
17+
isMultiSelect: boolean;
18+
shape: "circle" | "square";
19+
};
20+
const indicatorSpy = jest.spyOn(IndicatorComponent, "default");
21+
render(
22+
<Choice
23+
checked={false}
24+
indicatorContent="A"
25+
isMultiSelect={isMultiSelect}
26+
updateChecked={() => {}}
27+
>
28+
Option A
29+
</Choice>,
30+
);
31+
expect(indicatorSpy.mock.calls[0][0].shape).toBe(shape);
32+
},
33+
);
34+
35+
it.each`
36+
showCorrectness | stylingDescription
37+
${"correct"} | ${"proper styling for correctness"}
38+
${"wrong"} | ${"proper styling for wrongness"}
39+
${undefined} | ${"no additional styling when not in review mode"}
40+
`("applies $properOrNot styling $when", (args) => {
41+
const {showCorrectness} = args as {
42+
showCorrectness: "correct" | "wrong" | undefined;
43+
};
44+
const correctnessClasses = ["is-correct", "is-wrong"];
45+
const classToExpect = showCorrectness ? [`is-${showCorrectness}`] : [];
46+
const classesToNotExpect = correctnessClasses.filter(
47+
(className) => !classToExpect.includes(className),
48+
);
49+
render(
50+
<Choice
51+
checked={false}
52+
indicatorContent="A"
53+
isMultiSelect={false}
54+
showCorrectness={showCorrectness}
55+
updateChecked={() => {}}
56+
>
57+
Option A
58+
</Choice>,
59+
);
60+
const classes = Array.from(screen.getByRole("listitem").classList);
61+
expect(classes).toEqual(expect.arrayContaining(classToExpect));
62+
expect(classes).not.toEqual(expect.arrayContaining(classesToNotExpect));
63+
});
64+
65+
it.each`
66+
showCorrectness | doesOrDoesNot | callCount
67+
${"correct"} | ${"does NOT"} | ${0}
68+
${"wrong"} | ${"does NOT"} | ${0}
69+
${undefined} | ${"does"} | ${1}
70+
`(
71+
"click handler $doesOrDoesNot work when correctness is $showCorrectness",
72+
(args) => {
73+
const {showCorrectness, callCount} = args as {
74+
showCorrectness: "correct" | "wrong" | undefined;
75+
callCount: number;
76+
};
77+
const updateCheckedMock = jest.fn();
78+
render(
79+
<Choice
80+
checked={false}
81+
indicatorContent="A"
82+
isMultiSelect={false}
83+
showCorrectness={showCorrectness}
84+
updateChecked={updateCheckedMock}
85+
>
86+
Option A
87+
</Choice>,
88+
);
89+
const choiceElement = screen.getByRole("listitem");
90+
act(() => {
91+
choiceElement.click();
92+
});
93+
expect(updateCheckedMock).toHaveBeenCalledTimes(callCount);
94+
},
95+
);
96+
});
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
.choice {
2+
--perseus-multiple-choice-spacing: var(--wb-sizing-size_160);
3+
align-items: flex-start;
4+
border-bottom: var(--wb-border-width-thin) solid
5+
var(--wb-semanticColor-core-border-neutral-subtle);
6+
display: flex;
7+
font-size: var(--wb-font-heading-size-medium);
8+
/* "gap" is being used to put horizontal space between the indicator and the content.
9+
That way, we can keep all spacing in this file, and not have to apply styling to the indicator itself.
10+
*/
11+
gap: var(--perseus-multiple-choice-spacing);
12+
line-height: var(--wb-font-heading-lineHeight-medium);
13+
min-height: calc(var(--perseus-multiple-choice-spacing) * 4);
14+
padding: var(--perseus-multiple-choice-spacing);
15+
width: 100%;
16+
}
17+
18+
.choice:hover {
19+
cursor: pointer;
20+
}
21+
22+
/* Show a bottom border on the choice when:
23+
- the choice indicator has focus OR
24+
- the next choice indicator has focus OR
25+
- the choice is not in review mode AND
26+
- the choice is hovered OR
27+
- the next choice is hovered OR
28+
In other words, always show the bottom border when focus is on the indicator,
29+
but only show the bottom border for hover when not in review mode.
30+
*/
31+
.choice:focus-within,
32+
.choice:has(+ .choice:focus-within),
33+
.choice:not(.is-correct, .is-wrong) {
34+
&:hover,
35+
&:has(+ .choice:hover) {
36+
border-bottom: var(--wb-border-width-medium) solid
37+
var(--wb-semanticColor-core-border-neutral-default);
38+
padding-bottom: calc(
39+
var(--perseus-multiple-choice-spacing) - var(--wb-sizing-size_010)
40+
); /* Account for increased border width */
41+
}
42+
}
43+
44+
/* The first choice option needs additional styling for the border */
45+
.choice:first-of-type {
46+
border-top: var(--wb-border-width-thin) solid
47+
var(--wb-semanticColor-core-border-neutral-subtle);
48+
}
49+
50+
.choice:first-of-type:not(.is-correct, .is-wrong):hover,
51+
.choice:first-of-type:focus-within {
52+
border-top: var(--wb-border-width-medium) solid
53+
var(--wb-semanticColor-core-border-neutral-default);
54+
padding-top: calc(
55+
var(--perseus-multiple-choice-spacing) - var(--wb-sizing-size_010)
56+
); /* Account for increased border width */
57+
}
58+
59+
.is-correct {
60+
border: var(--wb-border-width-medium) solid
61+
var(--wb-semanticColor-core-border-success-default);
62+
border-radius: var(--wb-border-radius-radius_080);
63+
color: var(--wb-semanticColor-core-foreground-success-default);
64+
}
65+
66+
.is-wrong {
67+
color: var(--wb-semanticColor-core-foreground-critical-default);
68+
}
69+
70+
.is-correct:hover,
71+
.is-wrong:hover {
72+
cursor: default;
73+
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import React, {useRef} from "react";
2+
3+
import Indicator from "./choice-indicator.new";
4+
import styles from "./choice.module.css";
5+
6+
export interface IndicatorProps {
7+
checked: boolean;
8+
children: React.ReactNode | React.ReactNode[];
9+
indicatorContent: string; // This is the letter that shows for the choice
10+
isMultiSelect: boolean;
11+
showCorrectness?: "correct" | "wrong";
12+
updateChecked: (isChecked: boolean) => void;
13+
}
14+
15+
// TODO: Change name of this file to choice.new.tsx (replacing existing one)
16+
// and its corresponding test and Storybook files.
17+
18+
const Choice = (props: IndicatorProps) => {
19+
const showCorrectness = props.showCorrectness;
20+
const buttonRef = useRef<HTMLButtonElement>(null);
21+
const indicatorShape = props.isMultiSelect ? "square" : "circle";
22+
const clickHandler = showCorrectness
23+
? undefined
24+
: () => {
25+
buttonRef.current?.click();
26+
};
27+
const classes = [styles.choice]
28+
.concat(showCorrectness ? [styles["is-" + showCorrectness]] : [])
29+
.join(" ");
30+
return (
31+
/*
32+
A click handler is added to the <li> element to accommodate mouse users.
33+
Keyboard users will still be able to use the <button> element inside
34+
the <Indicator>.
35+
Therefore, WCAG 2.1.1 is still satisfied since functionality is
36+
aligned with the input method.
37+
*/
38+
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-noninteractive-element-interactions
39+
<li className={classes} onClick={clickHandler}>
40+
<Indicator
41+
buttonRef={buttonRef}
42+
checked={props.checked}
43+
content={props.indicatorContent}
44+
shape={indicatorShape}
45+
showCorrectness={showCorrectness}
46+
updateChecked={props.updateChecked}
47+
/>
48+
{props.children}
49+
</li>
50+
);
51+
};
52+
53+
export default Choice;

0 commit comments

Comments
 (0)