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.

Sign up for GitHub

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

Jump to bottom

UI 678 lm table #621

Merged
merged 13 commits into from
Aug 5, 2021
Merged

UI 678 lm table #621

merged 13 commits into from
Aug 5, 2021

Conversation

arb000r
Copy link
Collaborator

@arb000r arb000r commented Aug 3, 2021

Description

Fix warnings around the liquidity mining page. Add a banner to the top of the page with totals

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

How should this be tested?

Test banner works, mobile as well.
No warnings in console.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code where relevant, particularly in hard-to-understand areas
  • My changes generate no new console warnings
  • The base of this PR is master if hotfix, develop if not

@linear
Copy link

linear bot commented Aug 3, 2021

UI-678 Liquidity Mining table

Add liquidity mining page with liquidity mining distributions in a table

@vercel
Copy link

vercel bot commented Aug 3, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

kovan-app – ./

🔍 Inspect: https://vercel.com/balancer/kovan-app/2hTLkVm9z4Ku4du1FEByKii5TSEP
✅ Preview: Canceled

[Deployment for c4cf6eb canceled]

app – ./

🔍 Inspect: https://vercel.com/balancer/app/9Gk8VLQjMkX7HUPNCQMQ9qSHZEPb
✅ Preview: Canceled

[Deployment for c4cf6eb canceled]

staging-app – ./

🔍 Inspect: https://vercel.com/balancer/staging-app/7hir1Vse7BGGZpQG4EfKD7jrMjE1
✅ Preview: https://staging-app-git-ui-678-lm-table-balancer.vercel.app

staging-kovan-app – ./

🔍 Inspect: https://vercel.com/balancer/staging-kovan-app/FADNzo9qLSMtTaWEXZsV4UkcNkCG
✅ Preview: https://staging-kovan-app-git-ui-678-lm-table-balancer.vercel.app

gnosis – ./

🔍 Inspect: https://vercel.com/balancer/gnosis/H5VNLS74XLUQ9YmkUJGVX53LAn2Y
✅ Preview: https://gnosis-git-ui-678-lm-table-balancer.vercel.app

polygon – ./

🔍 Inspect: https://vercel.com/balancer/polygon/BciHsxPN9fohduzpnJi8pmZmKQdL
✅ Preview: https://polygon-git-ui-678-lm-table-balancer.vercel.app

beta-polygon – ./

🔍 Inspect: https://vercel.com/balancer/beta-polygon/HD7ZMJ9gm5WoFTRzEfn9ndu8VHpg
✅ Preview: https://beta-polygon-git-ui-678-lm-table-balancer.vercel.app

@vercel vercel bot temporarily deployed to Preview – app August 3, 2021 08:55 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app August 3, 2021 08:58 Inactive
garethfuller
garethfuller requested changes Aug 3, 2021
View reviewed changes
src/pages/LiquidityMining.vue Outdated Show resolved Hide resolved
src/pages/LiquidityMining.vue Outdated Show resolved Hide resolved
src/pages/LiquidityMining.vue Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – app August 4, 2021 09:12 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app August 4, 2021 09:12 Inactive
@vercel vercel bot temporarily deployed to Preview – app August 4, 2021 09:43 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app August 4, 2021 09:43 Inactive
@arb000r arb000r requested a review from garethfuller August 4, 2021 10:35
garethfuller
garethfuller approved these changes Aug 4, 2021
View reviewed changes
src/components/tables/LMTable/LMTable.vue
const usdValue =
prices.value[total.token.toLowerCase()].price * total.total;
totalUsd = totalUsd + usdValue;
const usdValue = priceFor(getAddress(total.token)) * total.total;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be picky but could we call this fiatValue

Suggested change
const usdValue = priceFor(getAddress(total.token)) * total.total;
const fiatValue = priceFor(getAddress(total.token)) * total.total;

garethfuller
garethfuller requested changes Aug 4, 2021
View reviewed changes
Copy link
Collaborator

@garethfuller garethfuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this doesn't seem to be working on mainnet

garethfuller
garethfuller approved these changes Aug 4, 2021
View reviewed changes
Copy link
Collaborator

@garethfuller garethfuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work now... 😅

@arb000r arb000r merged commit 55b8023 into develop Aug 5, 2021
TomAFrench added a commit that referenced this pull request Aug 7, 2021
@TomAFrench
Merge branch 'develop' into safe-app
04df591 
* develop:
  [Gnosis]: Add wrap/unwrap transaction action (#636)
  fix: use more accurate buy/sell amounts for transactions (#635)
  Add support for transaction speed ups / cancellations (#631)
  UI 678 lm table (#621)
  Cleanup dependencies (#629)
  Disable coingecko list for now (#628)
  Cleanup unused files & functions (#626)
  Hotfix/1.11.4 (#627)
  [Gnosis]: Various bug fixes (#624)
  Registry refactor hook up (#570)
  Persist transactions (#620)
  [Gnosis]: More UI Tweaks (#616)
This was referenced Aug 24, 2021
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garethfuller garethfuller garethfuller approved these changes

@evgenyboxer evgenyboxer Awaiting requested review from evgenyboxer

@pkattera pkattera Awaiting requested review from pkattera

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arb000r @garethfuller