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

feat(bridge-ui): more informative processing fee #13488

Merged

Conversation

jscriptcoder
Copy link
Contributor

@jscriptcoder jscriptcoder commented Mar 28, 2023

This PR addresses the following suggestions: #13476. Besides that there a few more things:

  • Type improvements:
    • Transaction data was a record of any. I added the correct types for this field.
    • Some chain ids were BigNumber type, which I found it unnecessary. They can simply be number.
  • Added a new component, NoticeModal, which presents the user with some message and the option to not seeing it again.
  • Grouped Transactions part, and done some improvements in the Transaction component. Still a lot to do here. I believe it's quite bloated. We could perfectly extract a lot of logic outside and also up in its parent such as claiming and releasing functions.
  • ProcessingFee component and related components have been grouped (we could move this to the root folder components)
    • Got rid of processingFee store. This is simply a bound option between parent and child. No need to be part of the global state.
  • Created a Button component to be consistent across the bridge, following a more declarative way to build UI. It's quite basic though. We need a design system here.
  • Other minor changes

@jscriptcoder jscriptcoder self-assigned this Mar 28, 2023
@jscriptcoder jscriptcoder linked an issue Mar 28, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Merging #13488 (2ba7871) into main (cfcf416) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main   #13488      +/-   ##
==========================================
- Coverage   36.14%   36.11%   -0.04%     
==========================================
  Files         124      124              
  Lines        3779     3777       -2     
  Branches      502      502              
==========================================
- Hits         1366     1364       -2     
  Misses       2321     2321              
  Partials       92       92              
Flag Coverage Δ *Carryforward flag
bridge-ui 94.18% <100.00%> (-0.03%) ⬇️
eventindexer ∅ <ø> (∅) Carriedforward from f255e72
protocol 0.00% <ø> (ø) Carriedforward from f255e72
relayer 62.53% <ø> (ø) Carriedforward from f255e72
ui 100.00% <ø> (ø) Carriedforward from f255e72

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
packages/bridge-ui/src/bridge/ERC20Bridge.ts 88.88% <ø> (ø)
packages/bridge-ui/src/domain/fee.ts 100.00% <ø> (ø)
packages/bridge-ui/src/domain/message.ts 100.00% <ø> (ø)
packages/bridge-ui/src/bridge/ETHBridge.ts 88.40% <100.00%> (-0.17%) ⬇️
packages/bridge-ui/src/storage/StorageService.ts 95.83% <100.00%> (-0.05%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cyberhorsey
Copy link
Contributor

It's failing some "unused import" lints as well but couldn't tag them in review

@jscriptcoder jscriptcoder dismissed cyberhorsey’s stale review March 30, 2023 07:43

It's not an error, but a warning. In any case I got rid of some of them... ESLint errors and warnings are getting addressed in another PR

@shadab-taiko
Copy link
Contributor

@jscriptcoder
The padding between the bridge button and memo radio changed
this is how it is now:
Screenshot 2023-03-30 at 7 54 26 PM

vs

how it used to be:
Screenshot 2023-03-30 at 7 54 33 PM

@jscriptcoder jscriptcoder added this pull request to the merge queue Mar 30, 2023
Merged via the queue into main with commit f5f7b7e Mar 30, 2023
@jscriptcoder jscriptcoder deleted the 13476-featbridge-ui-uiux-suggestion-to-better-inform-the-user branch March 30, 2023 14:30
@github-actions github-actions bot mentioned this pull request Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(bridge-ui): UI/UX suggestion to better inform the user
4 participants