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

Update ACHostConfig to use accessible TextBlock colors #3853

Merged
merged 11 commits into from
Apr 20, 2021

Conversation

corinagum
Copy link
Contributor

@corinagum corinagum commented Apr 16, 2021

Fixes #3812

Changelog Entry

  • Fixes #3812. Update adaptiveCardHostConfig to accessible text color-contrasts, by @corinagum in PR #XXX

Description

This updates the host config to use contrast-compliant colors.

I used colors from Fabric UI color scheme to match MSFT colors.

  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@corinagum corinagum changed the title 3812 color contrast Update ACHostConfig to use accessible TextBlock colors Apr 16, 2021
@corinagum corinagum marked this pull request as ready for review April 16, 2021 00:41
Copy link
Contributor

@compulim compulim left a comment

Choose a reason for hiding this comment

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

Few things:

  • Please run npm install. The screenshot generated are from AC 2.5.0, but not the latest AC 2.9.0
  • This file is not needed, probably generated because of earlier test failure: __tests__/__image_snapshots__/html/transcript-activity-grouping-js-transcript-with-activity-grouping-test-50-2-snap.png

CHANGELOG.md Show resolved Hide resolved
@corinagum corinagum enabled auto-merge (squash) April 16, 2021 21:49
@compulim
Copy link
Contributor

compulim commented Apr 20, 2021

Looks like there are some legit test failures.

For example, since a new message is added, the scroll bar size changed. The screenshot will need to be updated.

image

I believe running tests locally before submitting to CI, would catch these test failures sooner.

@corinagum
Copy link
Contributor Author

@compulim I'm running locally again to check, but this was most likely transient. I mistakenly added a command to the help command on MB that we don't want there, and then immediately removed it. I see it only failed once yesterday, around the time I made that push. The new messages button test only failed once, and the other repeating fails are timeout errors.

I'll report back once I run tests locally again, but I expect them all to pass.

@corinagum corinagum merged commit 26acedd into microsoft:master Apr 20, 2021
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.

color-contrast: Ensures the contrast between foreground and background colors meet ratio threshholds
2 participants