[Multichain] Fix: multichain disable cf creation #4208
Conversation
Branch preview✅ Deploy successful! Storybook: |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
| Page | Size (compressed) |
|---|---|
global |
979.3 KB (🟡 +25.06 KB) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Forty-two Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
| Page | Size (compressed) | First Load |
|---|---|---|
/ |
507 B (🟢 -24.38 KB) |
979.79 KB |
/403 |
574 B (🟢 -3 B) |
979.86 KB |
/404 |
697 B (🟢 -1 B) |
979.98 KB |
/_error |
230 B (🟡 +2 B) |
979.52 KB |
/_offline |
1.09 KB (🟢 -3 B) |
980.39 KB |
/addOwner |
535 B (🟡 +1 B) |
979.82 KB |
/address-book |
26.11 KB (🟡 +16 B) |
1005.41 KB |
/apps |
50.17 KB (🟢 -11 B) |
1.01 MB |
/apps/bookmarked |
446 B (🟡 +1 B) |
979.73 KB |
/apps/custom |
41.79 KB (🟢 -17 B) |
1021.09 KB |
/apps/open |
54.74 KB (🟡 +11 B) |
1.01 MB |
/balances |
30.66 KB (🟢 -340 B) |
1009.95 KB |
/balances/nfts |
19.16 KB (🟢 -19 B) |
998.46 KB |
/cookie |
8.76 KB (🟡 +27 B) |
988.06 KB |
/home |
60.3 KB (🟡 +85 B) |
1.02 MB |
/imprint |
1.38 KB (🟡 +2 B) |
980.68 KB |
/licenses |
4.97 KB (🟢 -3 B) |
984.27 KB |
/new-safe/advanced-create |
36.34 KB (🟡 +1.2 KB) |
1015.64 KB |
/new-safe/create |
35.59 KB (🟡 +1.18 KB) |
1014.89 KB |
/new-safe/load |
16.38 KB (🟢 -32 B) |
995.68 KB |
/privacy |
15.9 KB (🟡 +24 B) |
995.2 KB |
/settings |
450 B (🟢 -1 B) |
979.74 KB |
/settings/appearance |
8.02 KB (🟢 -20 B) |
987.32 KB |
/settings/cookies |
7.64 KB (🟢 -26 B) |
986.94 KB |
/settings/data |
7.54 KB (🟢 -22 B) |
986.84 KB |
/settings/environment-variables |
9.13 KB (🟢 -25 B) |
988.42 KB |
/settings/modules |
9.76 KB (🟢 -21 B) |
989.06 KB |
/settings/notifications |
27.85 KB (🟡 +835 B) |
1007.15 KB |
/settings/safe-apps |
25.55 KB (🟢 -13 B) |
1004.85 KB |
/settings/security |
8.06 KB (🟢 -25 B) |
987.36 KB |
/settings/setup |
35.96 KB (🟢 -24 B) |
1015.25 KB |
/share/safe-app |
9.72 KB (🟢 -3 B) |
989.02 KB |
/stake |
589 B (🟢 -13 B) |
979.87 KB |
/swap |
723 B (🟢 -9 B) |
980 KB |
/terms |
11.24 KB (🔴 +10.71 KB) |
990.54 KB |
/transactions |
73.47 KB (🟢 -48 B) |
1.03 MB |
/transactions/history |
73.44 KB (🟢 -49 B) |
1.03 MB |
/transactions/messages |
39.12 KB (🟢 -37 B) |
1018.42 KB |
/transactions/msg |
29.5 KB (🟢 -9 B) |
1008.8 KB |
/transactions/queue |
31.22 KB (🟢 -37 B) |
1010.52 KB |
/transactions/tx |
21.05 KB (🟢 -9 B) |
1000.35 KB |
/welcome |
6.77 KB (🟢 -4 B) |
986.07 KB |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.
Coverage report
Show files with reduced coverage 🔻
Test suite run success1527 tests passing in 206 suites. Report generated by 🧪jest coverage report action from aa02572 |
| if ((!allowNonOwner && !isSafeOwner && !isDelegate) || (isOnlySpendingLimit && !allowSpendingLimit)) { | ||
| return Message.NotSafeOwner | ||
| } | ||
| if (isUndeployedSafe && !allowUndeployedSafe) { | ||
| return Message.SafeNotActivated | ||
| } |
There was a problem hiding this comment.
I would reorder these two conditions to preserve the same order as previously since the check for NotSafeOwner is more specific than for SafeNotActivated.
Do we need the empty string return still or does it also work with undefined?
There was a problem hiding this comment.
We do not need an empty string, it also works with undefined:
if (!message) return children(true)
I'll adjust the order 👍
| const { container } = renderButton() | ||
|
|
||
| expect(container.querySelector('button')).toBeDisabled() | ||
| expect(container.querySelector('span[aria-label]')).toHaveAttribute( |
There was a problem hiding this comment.
you can use getByLabelText instead of querySelector. It's more performant than using query selectors: https://testing-library.com/docs/queries/bylabeltext/
There was a problem hiding this comment.
getByLabelText is nice, but in my opinion it makes tests brittle as a change to the text would cause the test to fail. Unless one explicitly cares about the button's text I wouldn't go for it. I personally prefer to have data-testid makes life way easier.
There was a problem hiding this comment.
I disagree. If possible we should select elements by their content as it reflects what the end user sees. If there is no content I would also fall back to using test ids. Imo its more brittle if we select by something like the HTML element. Changing the span to a p would make the test fail even though the element might still look the same to the user. This is also part of the recommendation from react-testing-library: https://testing-library.com/docs/queries/about/#priority
There was a problem hiding this comment.
This case especially is interested in seeing specific error messages in specific cases.
If we want to avoid it breaking if we just rephrase an error we could use constants for each individual error and use the same constants in the test.
There was a problem hiding this comment.
The only downside of getByLabelText vs querySelect is that the assertion error in case of a wrong expectation is worse:
With query selector:
Expected the element to have attribute:
aria-label="Wrong expectation"
Received:
aria-label="You need to activate the Safe before transacting"
With getByLabelText:
TestingLibraryElementError: Unable to find a label with the text of: Wrong expectation
Ignored nodes: comments, script, style
<div>
<span
aria-label="You need to activate the Safe before transacting"
class=""
data-mui-internal-clone-element="true"
>
<button
disabled=""
>
Continue
</button>
</span>
</div>
If the container is a big html element it becomes really hard to see why the field wasn't found.
What this PR changes
allowUndeployedSafeprop inCheckWalletHow to test it
Screenshots
Checklist