Skip to content
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

Migrate test off enzyme and karma #104

Merged
merged 9 commits into from
Jul 12, 2024
Merged

Migrate test off enzyme and karma #104

merged 9 commits into from
Jul 12, 2024

Conversation

jquense
Copy link
Member

@jquense jquense commented Jul 10, 2024

😥 This was more work than i was hoping for. But in a position to actually test out react 19

Switching tests from karma to vitest, and remove all the remaining enzyme

@jquense jquense requested a review from kyletsang July 10, 2024 21:24
Copy link
Collaborator

@kyletsang kyletsang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! A few items and that scrollbar test started failing again 🤔

test/AnchorSpec.tsx Outdated Show resolved Hide resolved
src/Overlay.tsx Outdated
@@ -222,7 +222,9 @@ const Overlay = React.forwardRef<HTMLElement, OverlayProps>(
onEntered,
});

return container ? ReactDOM.createPortal(child, container) : null;
return (
<>{container ? ReactDOM.createPortal(<>{child}</>, container) : null}</>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wrapping the portal + children in fragments required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for TS, but should be harmless i think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm unsure. I did pull down the branch and reverting the changes compiled for me. Maybe it conflicted with an earlier change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, nuking node_modules fixed it

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@jquense jquense merged commit d555a1f into main Jul 12, 2024
6 checks passed
@jquense jquense deleted the jq/tests branch July 12, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants