diff --git a/.changeset/dry-turtles-leave.md b/.changeset/dry-turtles-leave.md new file mode 100644 index 0000000000..ebf0de5464 --- /dev/null +++ b/.changeset/dry-turtles-leave.md @@ -0,0 +1,7 @@ +--- +"@khanacademy/perseus-dev-ui": patch +"@khanacademy/perseus": patch +--- + +[Storybook] Configure Aphrodite to Not Append !important to Styles +[Radio] Bugfix - Incorrect choice showing as blue instead of red diff --git a/.storybook/main.ts b/.storybook/main.ts index f8fd25a5e5..eb08bdbeb3 100644 --- a/.storybook/main.ts +++ b/.storybook/main.ts @@ -3,6 +3,25 @@ import {mergeConfig} from "vite"; import type {StorybookConfig} from "@storybook/react-vite"; +// This is a temporary plugin option to mimic what is in PROD in regard to cascade layers. +// Perseus LESS files are wrapped in the 'shared' layer in Webapp. +// To get the same ordering of precedence in Storybook, the imported LESS files need to be wrapped accordingly. +// Once the LESS files have cascade layers included (LEMS-2801), +// then the following plugin option should be removed. +const lessWrapper = { + name: "wrap-less-in-layer", + transform: (code: string, pathname: string) => { + if (pathname.endsWith(".less")) { + const layerStatements = + "@layer reset, shared, legacy;\n@layer shared"; + return { + code: `${layerStatements} { ${code} }`, + map: null, + }; + } + }, +}; + const config: StorybookConfig = { framework: "@storybook/react-vite", @@ -53,6 +72,7 @@ const config: StorybookConfig = { }, // Fix from: https://github.com/storybookjs/storybook/issues/25256#issuecomment-1866441206 assetsInclude: ["/sb-preview/runtime.js"], + plugins: [...viteConfig.plugins, lessWrapper], }); }, staticDirs: ["../static"], diff --git a/dev/vite.config.ts b/dev/vite.config.ts index b1f6268979..2fdf22f2da 100644 --- a/dev/vite.config.ts +++ b/dev/vite.config.ts @@ -27,6 +27,10 @@ export default defineConfig({ hubble: resolve(__dirname, "../vendor/hubble/hubble.js"), raphael: resolve(__dirname, "../vendor/raphael/raphael.js"), jsdiff: resolve(__dirname, "../vendor/jsdiff/jsdiff.js"), + aphrodite: resolve( + __dirname, + "../node_modules/aphrodite/no-important", + ), ...packageAliases, }, }, diff --git a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap index c81b660127..fdeff76800 100644 --- a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap +++ b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap @@ -189,10 +189,9 @@ exports[`group widget should snapshot: initial render 1`] = ` style="border-color: transparent; border-radius: 50%;" >
{ describe.each([[true], [false]])("multipleSelect: %s", (multipleSelect) => { - it("renders with the correct border radius", () => { + it("renders with the correct shape", () => { // Arrange - let expectedRadius; - if (multipleSelect) { - expectedRadius = 3; - } else { - expectedRadius = CHOICE_ICON_SIZE; - } + const expectedClass = multipleSelect + ? "multiSelectShape" + : "singleSelectShape"; // Act renderChoiceIcon({multipleSelect}); @@ -42,11 +38,10 @@ describe("choice icon", () => { const choiceWrapper = screen.getByTestId( `choice-icon__library-choice-icon`, ); + const choiceAttrib = choiceWrapper.getAttribute("class"); // Assert - expect(choiceWrapper).toHaveStyle( - `border-radius: ${expectedRadius}px`, - ); + expect(choiceAttrib).toContain(expectedClass); }); }); @@ -65,7 +60,7 @@ describe("choice icon", () => { const choiceAttrib = choiceWrapper.getAttribute("class"); // Assert - expect(choiceAttrib).toContain("circleCorrect"); + expect(choiceAttrib).toContain("choiceCorrect"); }); it("shows a dash for incorrect answers", () => { @@ -83,6 +78,6 @@ describe("choice icon", () => { const choiceAttrib = choiceWrapper.getAttribute("class"); // Assert - expect(choiceAttrib).toContain("circleIncorrect"); + expect(choiceAttrib).toContain("choiceIncorrect"); }); }); diff --git a/packages/perseus/src/widgets/radio/choice-icon/choice-icon.tsx b/packages/perseus/src/widgets/radio/choice-icon/choice-icon.tsx index 5f5e004090..2fe042cb66 100644 --- a/packages/perseus/src/widgets/radio/choice-icon/choice-icon.tsx +++ b/packages/perseus/src/widgets/radio/choice-icon/choice-icon.tsx @@ -57,50 +57,6 @@ function ChoiceInner(props: ChoiceInnerProps) { return ; } -// Handle dynamic styling of the multiple choice icon. Most -// MC icon styles are constant, but we do allow the caller -// to specify the selected color, and thus must control styles -// related to the selected state dynamically. -function getDynamicStyles( - checked: boolean, - showCorrectness: boolean, - pressed: boolean, - multipleSelect: boolean, - correct?: boolean | null, -): { - backgroundColor: string | undefined; - borderColor: string; - color: string; - borderRadius: number; -} { - let backgroundColor: string | undefined; - let borderColor: string; - let color: string; - if (!showCorrectness && pressed) { - borderColor = WBColor.blue; - color = WBColor.blue; - backgroundColor = "transparent"; - } else if (checked) { - const bg = showCorrectness && correct ? WBColor.green : WBColor.blue; - color = styleConstants.white; - backgroundColor = bg; - borderColor = bg; - } else { - borderColor = WBColor.offBlack64; - color = WBColor.offBlack64; - } - - // define shape - let borderRadius; - if (multipleSelect) { - borderRadius = 3; - } else { - borderRadius = CHOICE_ICON_SIZE; - } - - return {backgroundColor, borderColor, color, borderRadius}; -} - const ChoiceIcon = function (props: ChoiceIconProps): React.ReactElement { const { checked, @@ -111,17 +67,47 @@ const ChoiceIcon = function (props: ChoiceIconProps): React.ReactElement { hovered, multipleSelect, pos, - previouslyAnswered, + previouslyAnswered, // Used in "review mode"/"show rationale" to show that option was previously chosen pressed, } = props; - const dynamicStyles = getDynamicStyles( - checked, - showCorrectness, - pressed, - multipleSelect, - correct, - ); + // Accounts for incorrect choice still displaying as learner tries again + const choiceIsChecked = + checked || (showCorrectness && !correct && previouslyAnswered); + + // Core styling + const choiceStyling = [ + styles.choiceBase, + multipleSelect ? styles.multiSelectShape : styles.singleSelectShape, + showCorrectness ? styles.choiceHasIcon : styles.choiceHasLetter, + choiceIsChecked ? styles.choiceIsChecked : styles.choiceIsUnchecked, + ]; + + // Color styling + // Handle dynamic styling of the multiple choice icon. Most + // MC icon styles are constant, but we do allow the caller + // to specify the selected color, and thus must control styles + // related to the selected state dynamically. + let crossOutColor: string; + if (showCorrectness && correct && checked) { + choiceStyling.push(styles.choiceCorrect); + crossOutColor = WBColor.green; + } else if (showCorrectness && !correct && (checked || previouslyAnswered)) { + choiceStyling.push(styles.choiceIncorrect); + crossOutColor = WBColor.red; + } else if (checked) { + // Show filled neutral blue color (showCorrectness is false) + choiceStyling.push(styles.choiceNeutral); + crossOutColor = WBColor.blue; + } else if (pressed) { + // Show outlined neutral blue color (showCorrectness is false) + choiceStyling.push(styles.activeNeutral); + crossOutColor = WBColor.blue; + } else { + // choice is not checked + choiceStyling.push(styles.uncheckedColors); + crossOutColor = WBColor.offBlack64; + } return (
@@ -131,17 +117,8 @@ const ChoiceIcon = function (props: ChoiceIconProps): React.ReactElement { multipleSelect={multipleSelect} >
- {crossedOut && } + {crossedOut && }
); }; @@ -170,8 +147,7 @@ const styles = StyleSheet.create({ alignItems: "center", justifyContent: "center", }, - circle: { - // Make the circle + choiceBase: { width: CHOICE_ICON_SIZE, height: CHOICE_ICON_SIZE, boxSizing: "border-box", @@ -184,34 +160,60 @@ const styles = StyleSheet.create({ // "bold font family" so that characters which fall back to the default // font get bolded too. fontWeight: "bold", - fontSize: 12, // Center the icon wrapper. display: "flex", alignItems: "center", justifyContent: "center", - // HACK(emily): I don't know why adding this line height makes the text - // appear centered better than any other value, but it does. In - // particular, at large zoom levels this line height does almost - // nothing, but at the default size this shifts the letter down one - // pixel so it is much better centered. - lineHeight: "1px", }, - circleCorrect: { - fontSize: CHOICE_ICON_SIZE, + choiceHasLetter: { + fontSize: 12, }, - circleIncorrect: { + choiceHasIcon: { fontSize: CHOICE_ICON_SIZE, - borderColor: styleConstants.gray68, - color: styleConstants.gray68, }, - circleIncorrectAnswered: { + choiceIsChecked: { + color: WBColor.white, + }, + + choiceIsUnchecked: { + color: WBColor.offBlack64, + }, + + choiceCorrect: { + backgroundColor: WBColor.green, + borderColor: WBColor.green, + }, + + choiceIncorrect: { backgroundColor: WBColor.red, borderColor: WBColor.red, - color: WBColor.white, + }, + + choiceNeutral: { + backgroundColor: WBColor.blue, + borderColor: WBColor.blue, + }, + + activeNeutral: { + color: WBColor.blue, + borderColor: WBColor.blue, + backgroundColor: "transparent", + }, + + multiSelectShape: { + borderRadius: 3, + }, + + singleSelectShape: { + borderRadius: CHOICE_ICON_SIZE, + }, + + uncheckedColors: { + borderColor: WBColor.offBlack64, }, }); diff --git a/packages/perseus/src/widgets/radio/choice-icon/cross-out-line.tsx b/packages/perseus/src/widgets/radio/choice-icon/cross-out-line.tsx index ef8bf48b99..384a0740d7 100644 --- a/packages/perseus/src/widgets/radio/choice-icon/cross-out-line.tsx +++ b/packages/perseus/src/widgets/radio/choice-icon/cross-out-line.tsx @@ -43,8 +43,8 @@ const styles = StyleSheet.create({ crossOutLine: { // Center the icon within the container. position: "absolute", - top: `0px`, - left: `0px`, + top: `2px`, + left: `2px`, }, });