-
Notifications
You must be signed in to change notification settings - Fork 352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dropdown story with answerless data #2305
Conversation
Size Change: +30 B (0%) Total Size: 735 kB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (01e45d5) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2305 If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2305 |
…opdown that uses answerless data
@@ -41,3 +41,7 @@ export const DropdownWithEmptyPlaceholder = ( | |||
): React.ReactElement => { | |||
return <RendererWithDebugUI question={dropdownWithEmptyPlaceholder} />; | |||
}; | |||
|
|||
export const DropdownWithNoAnswers = (args: StoryArgs): React.ReactElement => { | |||
return <RendererWithDebugUI question={basicDropdown} answerless={true} />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: rather than add an answerless
prop to RendererWithDebugUI
, I might prefer to call splitPerseusItem
right here:
return <RendererWithDebugUI question={basicDropdown} answerless={true} />; | |
return <RendererWithDebugUI question={splitPerseusItem(basicDropdown)} />; |
That way, anyone reading the stories doesn't have to guess what answerless
does (or dig into RendererWithDebugUI
to figure it out) — they can see exactly what's happening. Granted, it would be clearer if splitPerseusItem
were called something like removeAnswers
...
I don't feel very strongly about this suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this suggestion! The simpler, the better I'd say. Not sure if we want to change the name of splitPerseusItem
, but I did add a doc string to explain what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooo, I think there's a problem doing it this way though. The problem no longer has access to the answer at all. When we use the answerless prop and pass the full question in, the answer is available for when the Check button is clicked. I forgot to confirm the Check button worked correctly. Going to revert back to the previous way, which allows the Check button to work.
Can just remove answers on the question data as it is being sent to the renderer. Extra prop not needed.
/** | ||
* Upgrades widget options and removes answerful data for all the widgets in a | ||
* Perseus item. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this comment!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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] ### Major Changes - [#2303](#2303) [`5e7a6084c`](5e7a608) Thanks [@benchristel](https://github.com/benchristel)! - Drop support for using KaTeX as a math renderer. You may encounter styling issues or TeX syntax errors if you try to implement `PerseusDependencies.TeX` using KaTeX. ### Minor Changes - [#2284](#2284) [`0d5ab0b2e`](0d5ab0b) Thanks [@nishasy](https://github.com/nishasy)! - Add new labelLocation value for Interactive Graphs - [#875](#875) [`9737eb497`](9737eb4) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove deprecated khan-exercise.css - [#2305](#2305) [`af6d89007`](af6d890) Thanks [@Myranae](https://github.com/Myranae)! - Add a story for Dropdown that uses answerless data ### Patch Changes - [#2307](#2307) [`dd7b13a78`](dd7b13a) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Revert SSS temporarily for Radio to ensure Multiselect + Random answer correctness is accurate - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Major Changes - [#2303](#2303) [`5e7a6084c`](5e7a608) Thanks [@benchristel](https://github.com/benchristel)! - Drop support for using KaTeX as a math renderer. You may encounter styling issues or TeX syntax errors if you try to implement `PerseusDependencies.TeX` using KaTeX. ### Minor Changes - [#2284](#2284) [`0d5ab0b2e`](0d5ab0b) Thanks [@nishasy](https://github.com/nishasy)! - Add new labelLocation value for Interactive Graphs ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea), [`9737eb497`](9737eb4), [`dd7b13a78`](dd7b13a), [`af6d89007`](af6d890), [`5e7a6084c`](5e7a608)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Minor Changes - [#2284](#2284) [`0d5ab0b2e`](0d5ab0b) Thanks [@nishasy](https://github.com/nishasy)! - Add new labelLocation value for Interactive Graphs ### Patch Changes - [#2300](#2300) [`fea65eaf1`](fea65ea) Thanks [@benchristel](https://github.com/benchristel)! - Bugfix: allow the 'key' field of Radio widgets to be null when parsing Perseus JSON ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`0d5ab0b2e`](0d5ab0b), [`fea65eaf1`](fea65ea)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected]
Summary:
Create a Storybook story for the Dropdown widget that renders with answerless data, is interactive, and is scorable.
Issue: LEMS-2930
Test plan: