-
Notifications
You must be signed in to change notification settings - Fork 364
feat: move batch execution out of experimental #3871
Conversation
* Removes batch execution toggle from settings * adds onboarding widget around batch execution button
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
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.
Great work dude! Couple of suggestions/questions.
src/routes/safe/components/Transactions/TxList/BatchExecute.tsx
Outdated
Show resolved
Hide resolved
src/routes/safe/components/Transactions/TxList/BatchExecuteButton.tsx
Outdated
Show resolved
Hide resolved
Pull Request Test Coverage Report for Build 2313147764
💛 - Coveralls |
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 work dude!
src/routes/safe/components/Transactions/TxList/BatchExecuteButton.tsx
Outdated
Show resolved
Hide resolved
onMouseEnter={handleOnMouseEnter} | ||
onMouseLeave={handleOnMouseLeave} | ||
> | ||
Execute Batch {isBatchable && `(${batchableTransactions.length})`} |
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.
Execute Batch {isBatchable && `(${batchableTransactions.length})`} | |
Execute Batch{isBatchable && ` (${batchableTransactions.length})`} |
src/routes/safe/components/Transactions/TxList/BatchExecuteButton.tsx
Outdated
Show resolved
Hide resolved
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.
Love the documentation!! Nice to see it popping when hovering the React component 👍
One semantic question... why did you opt to call it a widget? When I read OnboardingWidget it didn't take me to a tooltip
src/routes/safe/components/Transactions/TxList/BatchExecuteButton.tsx
Outdated
Show resolved
Hide resolved
src/routes/safe/components/Transactions/TxList/BatchExecute.tsx
Outdated
Show resolved
Hide resolved
* renames OnboardingWidget to OnboardingTooltip
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 work!
A tiny comment: can we make the "Got it!" button w/o a background/border? Like a link. |
@katspaugh done. I changed the button variant to "text". |
Top job! All good from my side ;) |
What it solves
Resolves #3857
How this PR fixes it
How to test it
Navigate to tx queue. Depending on how many signed txs the safe has the button will be enabled / disabled.
Initially the button will be promoted with a small tooltip until the user presses the "Got it" button
Analytics changes
Screenshots