Skip to content
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] - BUGFIX - Adjust color contrast for tooltip text #1966

Merged

Conversation

mark-fitzgerald
Copy link
Contributor

@mark-fitzgerald mark-fitzgerald commented Dec 6, 2024

Summary:

The text in the numeric input tooltip was slightly out of spec for accessibility. This change ensures that the text has a minimum 4.5:1 contrast ratio.

Test plan:

  1. Launch Storybook
  2. Navigate to Perseus Editor => Editor => Demo
  3. Add a Numeric Input widget
  4. Configure the widget to have any number of format options
  5. Place focus in the input field in the preview for the widget
  6. Check the color value in the browser inspector for the text in the tooltip (it should be greater than 4.5:1)

Affected UI:

Before

Color Contrast - Before

After

Color Contrast - After

@mark-fitzgerald mark-fitzgerald self-assigned this Dec 6, 2024
Copy link
Contributor

github-actions bot commented Dec 6, 2024

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (f36f5ef) and published it to npm. You
can install it using the tag PR1966.

Example:

yarn add @khanacademy/perseus@PR1966

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1966

Copy link
Contributor

github-actions bot commented Dec 6, 2024

Size Change: 0 B

Total Size: 1.29 MB

ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 39 kB
packages/keypad-context/dist/es/index.js 760 B
packages/kmath/dist/es/index.js 4.27 kB
packages/math-input/dist/es/index.js 77.9 kB
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-core/dist/es/index.js 1.48 kB
packages/perseus-editor/dist/es/index.js 697 kB
packages/perseus-linter/dist/es/index.js 22.2 kB
packages/perseus/dist/es/index.js 425 kB
packages/perseus/dist/es/strings.js 3.7 kB
packages/pure-markdown/dist/es/index.js 3.66 kB
packages/simple-markdown/dist/es/index.js 12.5 kB

compressed-size-action

@mark-fitzgerald mark-fitzgerald requested a review from a team December 6, 2024 22:35
@mark-fitzgerald mark-fitzgerald marked this pull request as ready for review December 6, 2024 22:35
@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Dec 6, 2024

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/poor-numbers-reflect.md, packages/perseus/src/styles/perseus-renderer.less, packages/perseus/src/styles/variables.less

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@@ -544,7 +544,7 @@

.perseus-formats-tooltip {
background: #fff;
color: #777;
color: #717378;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a WB color? Even though we're in a .less file, could you use (or define and use) a variable from variables.less?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is. I've moved that color to variables.less.

@nishasy
Copy link
Contributor

nishasy commented Dec 6, 2024

I trust this works perfectly, but for sake of PR completion and posterity, could you put a before and after screenshot here @mark-fitzgerald?

@mark-fitzgerald
Copy link
Contributor Author

I trust this works perfectly, but for sake of PR completion and posterity, could you put a before and after screenshot here @mark-fitzgerald?

Added.

Copy link
Contributor

@nishasy nishasy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🎉

@mark-fitzgerald mark-fitzgerald merged commit e22a931 into main Dec 6, 2024
20 of 21 checks passed
@mark-fitzgerald mark-fitzgerald deleted the bugfix/adjust-color-contrast-for-numeric-input-tooltip branch December 6, 2024 23:48
nishasy added a commit that referenced this pull request Dec 10, 2024
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]

### Patch Changes

- [#1966](#1966)
[`e22a931d9`](e22a931)
Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! -
[Numeric Input] - BUGFIX - Adjust color contrast of tooltip text

## @khanacademy/[email protected]

### Patch Changes

- [#1976](#1976)
[`2ad163b5e`](2ad163b)
Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Aria]
Update the auto-generated text to spell out commas

- Updated dependencies
\[[`e22a931d9`](e22a931)]:
    -   @khanacademy/[email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants