-
Notifications
You must be signed in to change notification settings - Fork 27
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
132. Upgrade Mantine packages #141
132. Upgrade Mantine packages #141
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
0aa068b
to
30f898f
Compare
apps/web/package.json
Outdated
"@mantine/form": "^7.5.0", | ||
"@mantine/hooks": "^7.5.0", | ||
"@mantine/notifications": "^7.5.0", | ||
"@mantine/core": "^7.7.0", |
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.
Just for info, mantine
is now at 7.7.1. Here's the change log. I see that the author(s) fixed some minor issues in that version, so maybe it's worth upgrading to it.
@@ -119,7 +125,7 @@ describe("VoucherExecution component", () => { | |||
expect(button.hasAttribute("disabled")).toBe(true); | |||
}); | |||
|
|||
it("should display enabled button when voucher is not yet executed", () => { | |||
it("should display enabled button when voucher is not yet executed", async () => { |
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, using async
and waitFor
should take care of the tooltip changes made on the mantine side, namely an async rendering of the tooltip. I don't think we need the timeout
configs though, I ran those tests locally without timeouts and they passed ok.
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.
Looking good, @dandheedge ! I made a couple of small suggestions for you to consider.
3e8a1ed
to
e0de0c7
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.
It looks good to me (deployed app + code). Also, I believe all the suggestions were implemented.
Close #132