-
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
[Numeric Input] - Update styling to match modern inputs #1832
base: main
Are you sure you want to change the base?
Conversation
…ts itself. Update snapshots to match.
Update snapshots accordingly.
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (cd0e0a6) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1832 If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1832 |
Size Change: +211 B (+0.02%) Total Size: 859 kB
ℹ️ View Unchanged
|
Adjust spacing.
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
fontSize: "18px", | ||
height: "32px", | ||
lineHeight: "18px", | ||
padding: this.state.isFocused ? "4px" : "4px 5px", // account for added focus border thickness | ||
textAlign: this.props.rightAlign ? "right" : "left", |
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.
Thank you for the comments to help explain the difference!
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.
This looks wonderful! :) Thank you for the great improvements
Summary:
The styling of the numeric input widget is quite constrained in regards to its size, and it is difficult to know when it has focus. Increasing its height will make it more physically accessible (better touch target). Adding more internal padding and increasing the border width will help with visual accessibility regarding a user's recognition of it having focus. Also, matching the styling that is used for the Expression widget (a close relative of this widget) lessens the cognitive load on users who see these two inputs as being similar/same (they don't know that they are two different widgets).
Issue: LEMS-2426
Test plan:
Affected UI:
Standard presentation
Before:
After:
In-Focus
Before:
After: