-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat/930-extend-toNewsKitIcon-types #931
Conversation
You can preview these changes on: |
@@ -46,6 +46,11 @@ const testsConfig = files | |||
disabledRules = disabledRulesObj.value.elements.map(node => node.value); | |||
} | |||
|
|||
if (title === 'cardcomposable') { |
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.
I've added these because the component loads dynamically brightcove video players with repeating aria IDs and this is flagged by the accessibility tests. I can see the same rules were disabled for "title": "video player" component, likely due to the same reason. Since this is loaded by a runtime script the test is flaky.
closes #930
What
Background - why this is needed
This is needed due to an update in the https://github.com/emotion-icons package. Bootstrapping a project with node 18 and react types 18 resolves the type EmotionIcon for the emotion icons. This results is a type error when passing them to the toNewsKitIcon transformer function.
What did you do
Extend toNewsKitIcon parameter types to include EmotionIcon.
What does the reviewers should expect
I have done:
I have tested manually:
Before:
After:
Who should review this PR:
How to test: