[Numeric Input] Update UI of editor #2015
Conversation
Move editor components to appropriate sections under headings.
…hose options. Update unit tests accordingly.
Convert user input of answer to be a single line for value and max error entry.
Group unsimplified options via fieldset Update unit tests accordingly.
…aid with unit tests. Update unit tests accordingly.
|
Size Change: +595 B (+0.04%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (884693f) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2015If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2015 |
| .inline-options { | ||
| float: inline-start; /* flexbox and inline-block don't work on <legend> elements, so going old-school here */ | ||
| line-height: 24px; /* for alignment with items in same line (like pills or buttons) */ | ||
| padding-inline-end: 0.5em; |
There was a problem hiding this comment.
Love these comments, thank you very much! It really helps with preventing regressions in the future imo.
| NumberInput, | ||
| TextInput, | ||
| } = components; | ||
| const {InfoTip, NumberInput, TextInput} = components; | ||
| const {firstNumericalParse} = Util; | ||
|
|
||
| // NOTE(john): Copied from perseus-types.d.ts in the Perseus package. |
There was a problem hiding this comment.
Total ignorable nit: I wonder if this comment should be updated with the recent changes from SSS. It's possible we might be able to import these types now rather than redeclare them.
| @@ -121,18 +121,45 @@ class NumericInputEditor extends React.Component<Props, State> { | |||
| super(props); | |||
| this.state = { | |||
| lastStatus: "wrong", | |||
| showOptions: _.map(this.props.answers, () => false), | |||
| showAnswerDetails: _.map(this.props.answers, () => true), | |||
There was a problem hiding this comment.
This seems like a good opportunity to remove another case of underscore!
| <span className="tooltip-for-legend"> | ||
| <InfoTip> | ||
| <p> | ||
| Normally select "ungraded". This |
There was a problem hiding this comment.
Is there something about our InfoTip that requires quotes to be designated in such a manner?
There was a problem hiding this comment.
Come to think of it, could this be an opportunity for the Wonderblocks tooltip, or do we have other restrictions that make that not feasible here? (I recall that we can't use it for the examples tooltip on the Widget itself, but I'm less sure about the Editor)
| size="medium" | ||
| role="radio" | ||
| style={{ | ||
| marginRight: "8px", |
There was a problem hiding this comment.
Ignorable nit: It seems like this style could potentially be pulled out into a variable as it's used 7 times. I see you've declared it once in SettingOption already. You could possibly set all these shared props and then add it to each pill.
| <div className="msg-title"> | ||
| Message shown to user on attempt | ||
| <Heading | ||
| title="General Settings" |
There was a problem hiding this comment.
I really love this separated section, it's much cleaner and helps Content Creators manage their screen space.
| <NumberInput | ||
| value={answer.value} | ||
| className="numeric-input-value" | ||
| placeholder="answer" |
There was a problem hiding this comment.
The most ignorable of nits: Should the "answer" placeholder be capitalized?
| )} | ||
| {answers[i]["status"] === "correct" && ( | ||
| <> | ||
| <legend className="inline-options"> |
There was a problem hiding this comment.
Nit: I'm noticing that this heading has the inline-options class, which pushes the (?) tooltip a little awkwardly to the right, while the "Possible answer formats" does not have this class. As a result, the balance between the two seems a little off.
Looking at the tooltips in General settings, I can see how the space is desirable for those cases. I wonder if we either want to remove the class from "Unsimplified answers are" or add it to "Possible answer formats", just for consistency.
| placeholder="answer" | ||
| format={_.last(answer.answerForms || [])} | ||
| onFormatChange={(newValue, format) => { | ||
| // NOTE(charlie): The mobile web expression |
There was a problem hiding this comment.
Nit: Is this comment still relevant? I'm pretty sure expression uses MathJax / MathIput now, and I don't see it being used for either the Widget or the Editor! We do still need Pi in NumericInput, though, so perhaps we could just update the comment.
| </fieldset> | ||
| {unsimplifiedAnswers(i)} | ||
| <div className="perseus-widget-row"> | ||
| Message shown to user in article: |
There was a problem hiding this comment.
I wish we had a way to identify which editor view we're in, as it'd be lovely to hide this option when adding Numeric Inputs in exercises.
Summary:
Many of the editor functions for Numeric Input were hidden behind a small configuration icon button, which was unintuitive. Also, the tooltip icons for the settings didn't align with their setting, making it difficult to understand where to get help for the options.
To fix these issues, and to modernize the UI, the editor screen was re-organized to make all functions easily findable. Also, Wonder Blocks components were used instead of custom items to make the editor experience consistent with other parts of the app.
Issue: LEMS-2456
Test plan:
Affected behavior:
Before
After