-
Notifications
You must be signed in to change notification settings - Fork 178
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(components): migrate LocationIcon to DeckInfoLabel #15846
Conversation
).withConfig({ | ||
shouldForwardProp: p => p !== 'oddStyle' && p !== 'desktopStyle', | ||
})` |
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.
@sfoster1 this un-forwards the non-style props and removes the console warnings Warning: React does not recognize the oddStyle prop on a DOM element.
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.
Blessed, thank you
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.
Looks good to me, thanks for fixing this!
).withConfig({ | ||
shouldForwardProp: p => p !== 'oddStyle' && p !== 'desktopStyle', | ||
})` |
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.
Blessed, thank you
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.
changes lgtm!
Overview
updates and renames the location icon component to reflect helix/ODD styles
closes PLAT-370, PLAT-408, RQA-2827
Test Plan and Hands on Testing
verified styles and updated unit tests
Changelog
Review requests
smoke test the various places the label appears, including deck map labels
Risk assessment
low