refactor(batch-selection): ts conversion - FE-4767 #5654
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4d9ea4f:
|
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| function renderBatchSelection( | ||
| props: Omit<BatchSelectionProps, "children">, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| renderer: any = mount |
There was a problem hiding this comment.
annoying that you have to find this any, but I can't find another good solution either. I guess we can live with it in a test suite (especially as we're going to move away from Enzyme anyway!). (Even typeof mount | typeof shallow doesn't work - TS seems to think you can't call that!)
There was a problem hiding this comment.
Yep, I spent some time trying to figure it out but then decided that it's not really worth it considering it's a spec file and as you said we'll be getting rid of enzyme anyawy
There was a problem hiding this comment.
one solution is to just remove the snapshot and not use shallow at all (we already have Chromatic for that). It will be removed anyway with our conversion to RTL.
| export const Light = Themed.bind({}); | ||
| Light.args = { | ||
| selectedCount: 2, | ||
| colorTheme: "light", | ||
| }; |
There was a problem hiding this comment.
I'm not sure why, but some of the onAction props in this example are being rendered as noRefCheck() which may be confusing. It seems modifying the story to not use args fixes the issue? I'm not sure why the first IconButton's onAction prop is being displayed correctly though.
This seems to be the case with the other stories using the Template.bind({}) pattern as well.
There was a problem hiding this comment.
That's weird I found an issue in Storybook github repo related to this - storybookjs/storybook#17701
It seems that it has already been fixed, but will be released in v7
storybookjs/storybook#19004
There was a problem hiding this comment.
Thanks for sharing this @mkrds 👍🏼 Its good to know this has been addressed by the storybook team.
As the release date for Storybook 7.0 doesn't seem to have been announced yet - what do you think about modifying the stories to not use the Template.bind() pattern in the meantime?
There was a problem hiding this comment.
Sure thing @Parsium, fixed :)
mkrds
force-pushed
the
FE-4767-batch-selection-ts-conversion
branch
3 times, most recently
from
172a40e to
371ebf1
Compare
mkrds
force-pushed
the
FE-4767-batch-selection-ts-conversion
branch
from
78dd6bc to
e534e43
Compare
mkrds
dismissed stale reviews from ZhuoyuJin, robinzigmond, and grabkowski
via
00d4374
|
🎉 This PR is included in version 112.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Proposed behaviour
This PR contains BatchSelection component TS conversion
Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions
Full regression testing of BatchSelection is recommended