box / box-content-preview

JavaScript library for rendering files stored on Box
https://developer.box.com/docs/box-content-preview
Other
106 stars 113 forks source link

chore(tests): Migrate from Enzyme to RTL (1st batch) #1535

Closed loonskai closed 3 weeks ago

loonskai commented 1 month ago

Description

Tests with Enzyme migrated: 23/61

This PR is a part of Box-wide migration to the newer React v18.

This PR will be followed with the next steps:

Screenshots

image
rafalmaksymiuk commented 1 month ago

This is a massive PR - tremendous work but hard to review. I know it is difficult to push each PR separately, but is way easier to meaningfully review so maybe split more next time :)?

Another thought - or rather question. I do know that EUA uses special tooling based on RTL. Don't remember details, but it integrates Redux somehow. Do you know why it was needed there and if it might be here?

nmkedziora commented 1 month ago

This is a massive PR - tremendous work but hard to review. I know it is difficult to push each PR separately, but is way easier to meaningfully review so maybe split more next time :)?

+1 for me. Amazing job and great effort 🎉

Splitting the PR by feature or folder would make a significant difference. It would not only make it easier to review but also easier for you to work on the changes (in terms of time and capacity) and quicker to merge bit by bit instead of blocking the merge for the entire PR due to comments on a single file.