Refactor: Box flex -> Stack #4629

katspaugh · 2024-12-09T12:25:22Z

Replace all instances of <Box display="flex" ... > with the more idiomatic and shorter <Stack ... >.

github-actions · 2024-12-09T12:25:38Z

Branch preview

✅ Deploy successful!

Website:
https://stack--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook:
https://stack--walletweb.review.5afe.dev/storybook/

github-actions · 2024-12-09T12:34:11Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	73.7% (-0.01% 🔻)	14368/19496
🔴	Branches	51.06% (-0.01% 🔻)	3414/6686
🔴	Functions	56.41% (-0.02% 🔻)	2028/3595
🟡	Lines	75.27% (-0.01% 🔻)	13038/17321

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / index.tsx	80.77% (-1.99% 🔻)	81.82%	60%	80% (-2.14% 🔻)
🟢	`...` / useTxPendingStatuses.ts	77.5% (-0.83% 🔻)	64.06% (-1.56% 🔻)	68.18%	86.41% (-0.97% 🔻)
🟢	`...` / index.tsx	80% (-5.71% 🔻)	100%	33.33% (-16.67% 🔻)	100%

Test suite run success

1666 tests passing in 227 suites.

Report generated by 🧪jest coverage report action from 68e5643

github-actions · 2024-12-09T13:16:07Z

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page	Size (compressed)
`global`	`1 MB` (🟡 +192 B)

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!

Twenty-six Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
`/_offline`	`1.05 KB` (🟢 -44 B)	1 MB
`/address-book`	`23.07 KB` (-1 B)	1.03 MB
`/apps`	`33.23 KB` (🟢 -29 B)	1.04 MB
`/apps/custom`	`31.29 KB` (🟢 -23 B)	1.03 MB
`/apps/open`	`53.65 KB` (-4 B)	1.06 MB
`/balances`	`29.83 KB` (🟡 +15 B)	1.03 MB
`/balances/nfts`	`9.62 KB` (🟢 -20 B)	1.01 MB
`/bridge`	`2.54 KB` (🟢 -8 B)	1.01 MB
`/home`	`58.77 KB` (🟡 +38 B)	1.06 MB
`/new-safe/advanced-create`	`26.45 KB` (🟡 +2 B)	1.03 MB
`/new-safe/create`	`25.58 KB` (🟡 +2 B)	1.03 MB
`/new-safe/load`	`6.06 KB` (🟡 +5 B)	1.01 MB
`/settings/data`	`1.88 KB` (🟡 +1 B)	1 MB
`/settings/modules`	`4.13 KB` (🟡 +1 B)	1.01 MB
`/settings/security`	`2.39 KB` (🟢 -22 B)	1.01 MB
`/settings/setup`	`30.76 KB` (🟢 -34 B)	1.03 MB
`/share/safe-app`	`7.56 KB` (🟢 -8 B)	1.01 MB
`/stake`	`607 B` (🟢 -2 B)	1 MB
`/transactions`	`96.91 KB` (🟡 +35 B)	1.1 MB
`/transactions/history`	`96.87 KB` (🟡 +34 B)	1.1 MB
`/transactions/messages`	`58.42 KB` (🟡 +34 B)	1.06 MB
`/transactions/msg`	`54.59 KB` (🟡 +38 B)	1.06 MB
`/transactions/queue`	`47.55 KB` (🟡 +36 B)	1.05 MB
`/transactions/tx`	`46.81 KB` (🟡 +35 B)	1.05 MB
`/welcome`	`6.8 KB` (🟡 +1 B)	1.01 MB
`/welcome/accounts`	`399 B` (🟢 -2 B)	1 MB

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.

github-actions

Code review by ChatGPT

github-actions · 2024-12-09T13:50:35Z

src/components/tx/SignOrExecuteForm/__tests__/__snapshots__/SignOrExecute.test.tsx.snap

@@ -206,7 +206,7 @@ exports[`SignOrExecute should display a loading component 1`] = `
      class="MuiCardContent-root cardContent css-1lt5qva-MuiCardContent-root"
    >
      <div
-        class="MuiBox-root css-17luq05"
+        class="MuiStack-root css-17uq0s2-MuiStack-root"
      >
        <div
          class="box MuiBox-root css-0"


Ensure that the class name change doesn't affect layout or styling elsewhere. Check for CSS specificity issues or dependencies that could arise from changing MuiBox-root to MuiStack-root. Verify no unintended UI changes occur with this update.

github-actions · 2024-12-09T13:50:35Z

...onents/tx/confirmation-views/BatchTransactions/__snapshots__/BatchTransactions.test.tsx.snap

@@ -146,7 +146,7 @@ exports[`BatchTransactions should render a list of batch transactions 1`] = `
                            class="MuiBox-root css-1lchl8k"
                          >
                            <div
-                              class="MuiBox-root css-171onha"
+                              class="MuiStack-root css-c4vnuj-MuiStack-root"
                              title="GnosisSafeProxy"
                            >
                              <div


Switching from MuiBox-root to MuiStack-root may affect layout or styling. Ensure the new component behaves as expected in all supported browsers, especially regarding flexbox behavior, as Stack relies heavily on it. Test thoroughly to avoid layout issues.

github-actions · 2024-12-09T13:50:35Z

src/components/tx/confirmation-views/SettingsChange/__snapshots__/SettingsChange.test.tsx.snap

@@ -240,7 +240,7 @@ exports[`SettingsChange should display the SettingsChange component with owner d
        class="MuiBox-root css-1lchl8k"
      >
        <div
-          class="MuiBox-root css-171onha"
+          class="MuiStack-root css-c4vnuj-MuiStack-root"
          title="Nevinha"
        >
          <div


No significant issues found in these specific changes. The alteration involves switching from MuiBox-root to MuiStack-root, which might reflect a layout or structural change. Verify if MuiStack is appropriate and aligns with intended design changes.

github-actions · 2024-12-09T13:50:35Z

src/components/tx/confirmation-views/__snapshots__/ConfirmationView.test.tsx.snap

@@ -937,7 +937,7 @@ exports[`ConfirmationView should display a confirmation with method call when th
                              class="MuiBox-root css-1lchl8k"
                            >
                              <div
-                                class="MuiBox-root css-171onha"
+                                class="MuiStack-root css-c4vnuj-MuiStack-root"
                                title="MetaMultiSigWallet"
                              >
                                <div


Ensure that the use of MuiStack-root instead of MuiBox-root is intentional and accounts for the layout and styling differences between these components. Check for potential impact on layout and visual consistency across different parts of the application.

usame-algan · 2024-12-09T15:29:34Z

src/components/balances/AssetsTable/index.tsx

@@ -185,7 +185,7 @@ const AssetsTable = ({
              sticky: true,
              collapsed: item.tokenInfo.address === hidingAsset,
              content: (
-                <Box display="flex" flexDirection="row" gap={1} alignItems="center">
+                <Stack direction="row" spacing={1} alignItems="center">


Looks like it didn't work here for some reason it keeps using Box.

I noticed a couple other places. Will close the PR, not worth the trouble.

katspaugh added 2 commits December 9, 2024 11:41

Refactor: Box flex -> Stack

4f14f1c

Memo

a66685f

katspaugh requested a review from usame-algan December 9, 2024 12:25

Fix tests

d532bc1

Lint

da10126

Update snapshots

68e5643

github-actions bot reviewed Dec 9, 2024

View reviewed changes

usame-algan reviewed Dec 9, 2024

View reviewed changes

katspaugh marked this pull request as draft December 10, 2024 08:05

katspaugh closed this Dec 10, 2024

katspaugh deleted the stack branch December 10, 2024 09:22

github-actions bot locked and limited conversation to collaborators Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: Box flex -> Stack #4629

Refactor: Box flex -> Stack #4629

katspaugh commented Dec 9, 2024

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot Dec 9, 2024

github-actions bot Dec 9, 2024

github-actions bot Dec 9, 2024

github-actions bot Dec 9, 2024

usame-algan Dec 9, 2024

katspaugh Dec 10, 2024

Refactor: Box flex -> Stack #4629

Refactor: Box flex -> Stack #4629

Conversation

katspaugh commented Dec 9, 2024

github-actions bot commented Dec 9, 2024 • edited Loading

Branch preview

github-actions bot commented Dec 9, 2024 • edited Loading

Coverage report

Test suite run success

github-actions bot commented Dec 9, 2024 • edited Loading

📦 Next.js Bundle Analysis for safe-wallet-web

⚠️ Global Bundle Size Increased

Twenty-six Pages Changed Size

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

usame-algan Dec 9, 2024

Choose a reason for hiding this comment

katspaugh Dec 10, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading