-
Notifications
You must be signed in to change notification settings - Fork 465
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
[Counterfactual] Add first steps widget #3218
Conversation
Branch preview⏳ Deploying a preview site... |
I think we can show this widget to any safe and add a close button in the corner to permanently hide it. cc @liliiaorlenko |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
09b309f
to
feb856b
Compare
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
Page | Size (compressed) |
---|---|
global |
1.02 MB (🟡 +67 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!
Five Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
Page | Size (compressed) | First Load |
---|---|---|
/home |
81.19 KB (🔴 +40.38 KB) |
1.1 MB |
/new-safe/create |
30.9 KB (-1 B) |
1.05 MB |
/new-safe/load |
19.19 KB (🟡 +49 B) |
1.04 MB |
/settings/security-login |
29.6 KB (🟢 -4 B) |
1.05 MB |
/settings/setup |
71.32 KB (🟡 +643 B) |
1.09 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.
Coverage report
Show files with reduced coverage 🔻
Test suite run success1389 tests passing in 188 suites. Report generated by 🧪jest coverage report action from b86b66c |
const totalNumberOfItems = items.length | ||
const completedItems = items.filter((item) => item.completed) | ||
const progress = Math.round((completedItems.length / totalNumberOfItems) * 100) |
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.
These three vars are only needed to compute the progress
var. Extract them into a calculateProgress
function?
const items = [ | ||
{ name: 'Add funds', completed: hasNonZeroBalance }, | ||
{ name: 'Create your first transaction', completed: hasOutgoingTransactions }, | ||
{ name: 'Safe is ready', completed: hasNonZeroBalance && hasOutgoingTransactions }, | ||
] |
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.
I would
- define the names of steps as a sepate enum
- memoize this object
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.
I also removed the third step as its more like a success step when everything else is completed and the progress bar is more accurate without. I added it as an additional prop to StatusProgress
instead.
</StatusItem> | ||
) | ||
})} | ||
{completedName && <StatusItem completed={progress === 100}>{completedName}</StatusItem>} |
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.
Aren't all items supposed to be shown at all times? And if not completed it's grey?
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.
On secound thought I think I will remove this again because its very specific for the counterfactual flow where we want to show the user the goal of these steps "Safe is ready" and there is an alternative design for that which I am planning to implement.
@katspaugh I implemented the alternative design with some placeholder values for counterfactual so I would wait to merge this until #3180 is merged. |
3fe1dd8
to
c6cccc1
Compare
ESLint Summary View Full Report
Report generated by eslint-plus-action |
34c56e0
to
b86b66c
Compare
This ticket was tested and looks good, but with Usame we have decied to merge this ticket along with 3220 and 3214 since these 3 are piceces of a functionality, and it is easier to test all together. |
5c4e44d
to
0804a08
Compare
What it solves
Part of #3207
How this PR fixes it
StatusProgress
component that can be reused for other widgetsHow to test it
Screenshots
Checklist