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
chore: upgrade styled-components to v5 #2865
Conversation
Size Change: 0 B Total Size: 482 kB ℹ️ View Unchanged
|
4fe8b30
to
bebb8a9
Compare
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.
EDIT: I just now realized that this is WIP. 🤦 It's my fault, but what will help me is if you use GitHub's "draft" feature instead of the label. 🙏
Regarding the failing tests, try to work around that limitation, either by implementing the solution in the comment you linked to, or by avoiding the assertions that are failing in one of the following ways:
- in some cases it might make sense to do
expect(screen.getByTestId(...)).toMatchInlineSnapshot()
, for example inpackages/orbit-components/src/Mobile/__tests__/index.test.js
, where the styles happen to be pretty much only what you're interested in anyway, nothing extra - some of the failing
Popover
tests seem to lack context, it's not obvious to me what they are ensuring, maybe that's only because the test names could be better, but it makes sense to test particular helpers instead - in some tests where you don't need
modifier
, like insrc/Stack/__tests__/index.test.js
, you might be able to make assertions usinggetComputedStyle
instead oftoHaveStyleRule
- finally, I don't think it's terrible to temporarily disable particular tests if our stack is failing us, besides, thoroughly testing styles is not a job for unit tests anyway
Fortunately there's not so many of them. 🤞
<Popover opened offset={{ top: 10, left: 20 }} content="kek"> | ||
<Button type="secondary" size="small"> | ||
Cancel | ||
</Button> | ||
</Popover>, | ||
); | ||
|
||
debug(); |
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 assume you forgot to delete this. 😃
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 know about draft
, i thought it's ready, but then it's started to fail again and i know why, so i changed it to WIP
, in order to prevent you from reviewing not finished PR :D
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.
thanks a lot for suggestion :) applied inline snapshot
to Mobile
and removed that media
check for Popover
, seems like a redundant check, I think it's enough to check if the offset
is passed
bebb8a9
to
77468a0
Compare
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.
Nice one! 🚀
chore: update attrs to SC5 fix(tests): fix failing tests with new SC5
77468a0
to
3c4fcd5
Compare
Orbit.kiwi: https://orbit-docs-chore-upgrade-sc-to-version-5.surge.sh
Storybook: https://orbit-chore-upgrade-sc-to-version-5.surge.sh