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

fix(deprecatedrole,color-contrast): fix message to properly include deprecated role, improve color-contrast pass messages #3387

Merged
merged 4 commits into from
Feb 28, 2022

Conversation

straker
Copy link
Contributor

@straker straker commented Feb 7, 2022

Missed during review (my fault) that data.values wouldn't work for non-array data string. Might try my hand at extending our check message validation script to include verifying that the data object matches all interpolated parts of the string (probably using process-message)

Closes issue: #3384

@straker straker requested a review from a team as a code owner February 7, 2022 17:02
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

I was thinking about this the other day. We don't really have a way to test this, which has caused a few issues. How about if we add something to testUtils.MockCheckContext that lets us write tests for these messages?

@straker straker marked this pull request as draft February 11, 2022 16:26
@straker straker marked this pull request as ready for review February 18, 2022 17:32
@straker
Copy link
Contributor Author

straker commented Feb 18, 2022

Doing this revealed that color-contrast had a few pass cases where the message wasn't passed any data. Fixed those with the test.

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

LGTM, just update the title, since you're not just touching deprecatedrole.

@straker straker changed the title fix(deprecatedrole): fix message to properly include deprecated role fix(deprecatedrole,color-contrast): fix message to properly include deprecated role, improve color-contrast pass messages Feb 24, 2022
@straker straker merged commit c8b4b27 into develop Feb 28, 2022
@straker straker deleted the deprecatedrole-msg branch February 28, 2022 16:09
straker added a commit that referenced this pull request May 12, 2022
…eprecated role, improve color-contrast pass messages (#3387)

* fix(deprecatedrole): fix message to properly include deprecated role

* add tests

* ie11
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.

2 participants