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(bridge-ui): transaction tab same width as bridge width #8195

Merged
merged 7 commits into from
Jan 10, 2023

Conversation

wolfderechter
Copy link
Contributor

The transacton tab now has the same width as the bridge tab as requested per #7730.
image
image

@codecov
Copy link

codecov bot commented Jan 4, 2023

Codecov Report

Merging #8195 (28454bb) into main (362d083) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #8195   +/-   ##
=======================================
  Coverage   67.58%   67.58%           
=======================================
  Files         109      109           
  Lines        2934     2934           
  Branches      354      354           
=======================================
  Hits         1983     1983           
  Misses        875      875           
  Partials       76       76           
Flag Coverage Δ *Carryforward flag
bridge-ui 95.05% <ø> (ø)
protocol 60.92% <ø> (ø) Carriedforward from 362d083
relayer 69.10% <ø> (ø) Carriedforward from 362d083
ui 100.00% <ø> (ø) Carriedforward from 362d083

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

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

@wolfderechter
Copy link
Contributor Author

Cc @dantaik @shadab-taiko

dantaik
dantaik previously approved these changes Jan 5, 2023
@dantaik
Copy link
Contributor

dantaik commented Jan 5, 2023

@shadab-taiko can we also make the min-height almost the same as the other tab?

@wolfderechter
Copy link
Contributor Author

image
I was thinking about that before but didn't implement it. I added it now

@shadab-taiko
Copy link
Contributor

shadab-taiko commented Jan 5, 2023

Screenshot 2023-01-05 at 7 22 02 PM

@wolfderechter This breaks once the transactions are populated.

@dong77
Copy link
Contributor

dong77 commented Jan 5, 2023

Looks terrible

@dong77
Copy link
Contributor

dong77 commented Jan 5, 2023

We can remove the two layer-icons from the table and change "Claimed" to a green check mark.
We can even merge From to into one column so "Ethereum -> Taiko" takes less space.

@wolfderechter
Copy link
Contributor Author

@shadab-taiko you are right, I couldn't populate it locally but I think I fixed it. Do you know how I can populate locally? I think my .env isn't correct

@dong77
Copy link
Contributor

dong77 commented Jan 5, 2023

If "claim" means an action, it should look like a button or have a different color.

@wolfderechter
Copy link
Contributor Author

I think I got it, I set the min-height to that of the bridge tab when there are 0 transactions.
taikobridgenotransactions

And I don't set a min-height when there are transactions so it stays fit-content. The min-width gets set based on the bridge width but the width increases to fit-content when there are transactions.
taikobridgetransactions
@dantaik what do you think?

Perhaps in another pull request the 'From and To' tab can be merged, my solution should still work when that happens.

Let's hope my solution works in the testing environment as I'm not 100% sure.

@vercel
Copy link

vercel bot commented Jan 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
website 🔄 Building (Inspect) Jan 5, 2023 at 2:56PM (UTC)

@dantaik
Copy link
Contributor

dantaik commented Jan 6, 2023

LGTM

@dantaik
Copy link
Contributor

dantaik commented Jan 6, 2023

@shadab-taiko could you please verify if everything works?

@shadab-taiko
Copy link
Contributor

Verified the transactions list with and without transactions. LGTM 🚀 .

Thanks @wolfderechter

@dantaik dantaik merged commit 85a5bfd into taikoxyz:main Jan 10, 2023
@github-actions github-actions bot mentioned this pull request Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants