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

fix: Don't break array elements in tx details into multiple lines if empty #4571

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

usame-algan
Copy link
Member

What it solves

In case a transaction method parameter is an empty array we should not break the brackets into multiple lines to improve readability.

How this PR fixes it

  • Only renders the div for the array contents if its not empty

How to test it

  1. Open a transaction that has array method parameters that are empty
  2. Observe the array brackets don't break into the next line

Screenshots

Before:
Screenshot 2024-11-26 at 15 50 11

After:
Screenshot 2024-11-26 at 15 50 45

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@usame-algan usame-algan changed the title fix: Don't break array elements in tx details into multiple lines if … fix: Don't break array elements in tx details into multiple lines if empty Nov 26, 2024
Copy link

📦 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.01 MB (🟡 +11 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!

Copy link

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 73.96% 14128/19103
🔴 Branches
52.07% (-0.01% 🔻)
3502/6726
🔴 Functions 57.59% 2063/3582
🟡 Lines 75.59% 12835/16979
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟡
... / index.tsx
67.74%
14.29% (-1.1% 🔻)
71.43% 66.67%

Test suite run success

1622 tests passing in 217 suites.

Report generated by 🧪jest coverage report action from c752fe8

@katspaugh katspaugh merged commit 047f45d into dev Nov 28, 2024
15 checks passed
@katspaugh katspaugh deleted the tx-details-empty-array-values branch November 28, 2024 13:32
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2024
@katspaugh
Copy link
Member

Tested ✅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants