Switch out app utilities with AragonUI utilities #1166

ECWireless · 2020-06-15T17:24:37Z

This PR is a work in progress.

Utilities to import from AragonUI:

shortenAddress()
addressesEqual()
formatTokenAmount() in place of formatNumber()

* Copy improvements * More specific copy * Reverting controversial change

* Change confusing token manager copy

* voting: fetch active account votes in script * Move connectedAccount out of the store and cache * Add comment about using strings over symbols * Move functions to helpers * Update comments Co-Authored-By: 2color <[email protected]> * Add newline

Improve the filters layout: - Move TransfersFilters to its own component. - Make the layout work at all sizes. - Update the download button so it looks like the other icon buttons. - Update some labels: - “Date range” => “Period” - “Transfer type” => “Type” (since it is already below the “Transfers” title). - Select the last 90 days (rolling) by default.

* Voting: fix bug with votes not loading initially * Add comment

Fixes aragon/client#734. With aragon/aragon.js#277 we now get the actual errors back from the RPC when a call goes wrong (e.g. naive `token.symbol()` on DAI) and we need to handle these explicitly. This PR converts a lot of the token fallback-related bits into simpler Promise-based code and adds explicit handlers for their error cases.

* TransferFilters: remove duplicate margin rules * DatePicker: update today border color * DateRangePicker: update to allow null values on dates * Transfers: refactor to no default date selection * DateRangeInput: fix font family declaration Co-Authored-By: AquiGorka <[email protected]> * Transfers: recover non-breaking space * DateRangeInput: improve placeholder logic and add input placeholder for screen readers * DateRangeInput: improve on placeholder for accesibility

…ragon#822) * feat(Payroll): small SLOAD optimizations to _transferTokensAmount * fix: put back accidentally deleted line from merge conflict

) Votes can sometimes be created in the same block, and so will have the same start (and end time) attached. In this case, we should use their `voteId`s for sorting, so that the vote numbers still look correct (as you would expect a "newer" vote, with a larger `voteId`, to have a later end date).

Prevents the app to crash on the current Firefox Nightly.

* VotingCard: update informative to question * Math utils: refactor scale values set to use bn * Add jest and babel-jest deps for tests * babelrc: add env tests presets * Add math utils tests

…n#830)

Noticeable improvement: the side panels transition should be smoother. Changes: - Move panels in their own components, so that the rendering of SidePanel can be skipped entirely. - Memoize the object returned by usePanel(), so that it can be compared using a strict equality. - Move the voting panel actions in their own component fetching their data, so that the panel component doesn’t need to render that much anymore. - Don’t recreate `votes` every second, instead only do it if any “opened” status has changed.

aragon#1171)

* Upgrade aragonOS dependency For Vault, Agent and Finance. To fix Istanbul related issues. See https://github.com/aragon/aragonOS/releases/tag/v4.3.0 for more context. * payroll: fix gas values in tests * agent: fix tests `isValidSignature` was calling the `bytes` version due to overloading issue with Truffle. * Fix CI, address PR aragon#1172 comments

- Also, remove addressesEqual() from web3-utils.js

…ount() - Also, remove formatAmount() from math-utils.js

ECWireless · 2020-06-18T18:08:01Z

@sohkai These are the only duplicated utilities I could find, but let me know if there are others I can add to the list!

Also, I couldn't find any way to install the Survey app (?), so I've skipped replacing formatAmount()in that app for these commits.

sohkai · 2020-06-19T08:42:07Z

Also, I couldn't find any way to install the Survey app (?), so I've skipped replacing formatAmount()in that app for these commits.

No worries, we are removing the Survey app from this repo soon anyway :). Thanks for doing this!

sohkai

Looking really good @ECWireless! Perhaps @bpierre or @Evalir can do a quick scan to check if we have any other utilities that can be deduped, but otherwise this is great!

I've also left a few comments for us to work out and hopefully get this merged soon!

sohkai · 2020-06-19T08:45:53Z

apps/voting/app/src/components/SummaryRows.js

@@ -1,8 +1,7 @@
 import React from 'react'


Ah, sorry for the rework—we are likely to have a conflict here against #1113.

We'll merge that one first, and then it should be pretty easy to rollback these changes against Voting here to solve any conflicts :).

sohkai · 2020-06-19T08:48:10Z

apps/token-manager/app/src/script.js

@@ -1,6 +1,6 @@
 import Aragon, { events } from '@aragon/api'
+import { addressesEqual }  from '@aragon/ui'


Ah... this may be a problem @bpierre.

This is still run in the context of a browser, but I think we should aim for this script to be runnable in a node / etc. environment. AFAIK nothing else used by the script should cause an issue.

From what I remember, importing @aragon/ui may force a browser context?

It should be fine, aragonUI is now compatible with non-DOM environments: https://github.com/aragon/aragon-ui/pull/699

But another thing to take care of would be to ensure that the entire library doesn’t get included in script.js. That would require using --experimental-scope-hoisting, but we were having issues with it IIRC.

What would you guys think of inlining the function in every script.js using it, until we move to Parcel 2 or another bundler with a good support for tree shaking? https://github.com/aragon/aragon-ui/blob/master/src/utils/web3.js#L82-L92

apps/token-manager/app/src/screens/Holders.js

* Fix finance USD conversion mechanism * Add Jest * Simplify conversion function * Add some tests for the conversion utility Co-authored-by: Pierre Bertet <[email protected]>

Evalir

Looking awesome @ECWireless ! Checked for any utilities left and didn't see any that we could move for now. Did some QA over the apps and everything seems to be consistent. Added 2 commits for:

Bumping Voting's @aragon/ui version to 1.4.2, as the previous one didn't include formatTokenAmount and was thus breaking the app. (Will be added by Voting: use BN.js everywhere #1113 as well)
Re-adding date formatting changes added in a previous commit by another contributor. This was probably due to switching branches on a commit that didn't have them!
Fixing some small typos. :)

Pretty confident this should be good to go now, but if you have any time @bpierre you can take a look! We will still have to wait for #1113 though.

coveralls · 2020-06-26T02:59:40Z

Coverage remained the same at 97.642% when pulling 77234e2 on ECWireless:ecwireless-add-aragonui-utilities into 41ac706 on aragon:master.

bpierre · 2020-06-19T13:27:13Z

apps/finance/app/src/components/useFilteredTransfers.js

@@ -6,7 +6,7 @@ import {
  Incoming,
  Outgoing,
 } from '../transfer-types'
-import { addressesEqual } from '../lib/web3-utils'
+import { addressesEqual} from '@aragon/ui'


Suggested change

import { addressesEqual} from '@aragon/ui'

import { addressesEqual } from '@aragon/ui'

bpierre · 2020-07-01T20:13:18Z

apps/token-manager/app/src/script.js

@@ -1,6 +1,6 @@
 import Aragon, { events } from '@aragon/api'
+import { addressesEqual }  from '@aragon/ui'


It should be fine, aragonUI is now compatible with non-DOM environments: https://github.com/aragon/aragon-ui/pull/699

But another thing to take care of would be to ensure that the entire library doesn’t get included in script.js. That would require using --experimental-scope-hoisting, but we were having issues with it IIRC.

What would you guys think of inlining the function in every script.js using it, until we move to Parcel 2 or another bundler with a good support for tree shaking? https://github.com/aragon/aragon-ui/blob/master/src/utils/web3.js#L82-L92

facuspagnuolo and others added 30 commits April 16, 2019 21:11

Payroll: Fix exchange rates request call (aragon#797)

57d8978

Copy improvements (aragon#795)

b2fcc4d

* Copy improvements * More specific copy * Reverting controversial change

Replace confusing Token Manager copy (aragon#807)

9b4bb77

* Change confusing token manager copy

Apps: Use shared TimeHelpersMock for tests (aragon#786)

92fd5e5

Voting: clarify polls (aragon#808)

231a43b

Payroll: Add reentracy guards (aragon#778)

eaf09e7

Finance: fix the download button (aragon#810)

0ddce97

Voting: fix bug with votes not loading initially (aragon#812)

0bf6ff4

* Voting: fix bug with votes not loading initially * Add comment

Finance: upgrade @aragon/templates-tokens (aragon#815)

0183acd

chore: remove unnecessary dependencies (aragon#817)

5eb9872

Apps: Tests enhancements (aragon#788)

65a998f

Token Manager: fix copy (aragon#811)

41a08a4

Finance: fix double border on balance (aragon#818)

82c3020

chore: fix solidity-coverage in apps (aragon#820)

8f0b12b

shared-test-helpers: v1.2.1

dbf5366

feat(Payroll): small SLOAD optimizations to _transferTokensAmount() (a…

5f190c5

…ragon#822) * feat(Payroll): small SLOAD optimizations to _transferTokensAmount * fix: put back accidentally deleted line from merge conflict

Finance: prevent RangeError when .toLocaleString() is used (aragon#827)

4fdd2f0

Prevents the app to crash on the current Firefox Nightly.

Fix ganache-cli.sh on Ubuntu (aragon#826)

2f76b6c

shared-test-helpers: v1.2.2

cb9b20c

Payroll: clarify how rates are handled and add unit tests (aragon#823)

327abd6

Fix voting displayed percent (aragon#825)

8edc217

* VotingCard: update informative to question * Math utils: refactor scale values set to use bn * Add jest and babel-jest deps for tests * babelrc: add env tests presets * Add math utils tests

Voting: prevent cards to get larger than the viewport (aragon#828)

7d13dd8

Update App Center copy for core apps (aragon#784)

1c32e8f

feat(Payroll): include exchange rate used in SendPayment event (arago…

1f34d07

…n#830)

sohkai and others added 5 commits June 16, 2020 12:41

Voting: fix typo in app center description (aragon#1168)

e5b81b5

Tokens: Move renderEntry hook calls into its own component & fix crash (

911a104

aragon#1171)

Agent: use @aragon/ui version of addressesEqual()

3df02e6

- Also, remove addressesEqual() from web3-utils.js

Finance: use @aragon/ui version of addressesEqual()

12924bb

- Also, remove addressesEqual() from web3-utils.js

probot-autolabeler bot added the app: finance label Jun 18, 2020

ECWireless added 4 commits June 18, 2020 10:11

Token-Manager: use @aragon/ui version of addressesEqual()

823eb37

Token Manager: remove addressesEqual() from web3-utils.js

5ba1ae6

Voting: use @aragon/ui version of addressesEqual()

e706887

- Also, remove addressesEqual() from web3-utils.js

Voting: use @aragon/ui formatTokenAmount() rather than local formatAm…

758cd8a

…ount() - Also, remove formatAmount() from math-utils.js

ECWireless marked this pull request as ready for review June 18, 2020 18:08

auto-assign bot requested review from bpierre and Evalir June 18, 2020 18:08

sohkai reviewed Jun 19, 2020

View reviewed changes

ECWireless and others added 5 commits June 22, 2020 11:22

Move utility to end of import list

cbf077b

Agent, Finance: update date format (aragon#1174)

3906547

Agent: update help text (aragon#1169)

b9a4de1

Fix finance USD conversion mechanism (aragon#1177)

41ac706

* Fix finance USD conversion mechanism * Add Jest * Simplify conversion function * Add some tests for the conversion utility Co-authored-by: Pierre Bertet <[email protected]>

Bump up Voting's @aragon/ui to 1.4.2 (latest), fix typo in package.json

548a172

Evalir force-pushed the ecwireless-add-aragonui-utilities branch from 6c5d3f2 to 548a172 Compare June 26, 2020 01:20

Evalir and others added 2 commits June 25, 2020 21:22

Merge branch 'master' into ecwireless-add-aragonui-utilities

b50a401

Re-add date changes

77234e2

Evalir approved these changes Jun 26, 2020

View reviewed changes

bpierre reviewed Jul 1, 2020

View reviewed changes

WissenIstNacht closed this Sep 29, 2021

WissenIstNacht force-pushed the master branch from 6e14239 to f7ef93b Compare September 29, 2021 09:52

theobayard mentioned this pull request Nov 18, 2021

chore: prefer utilities provided by aragonUI #1156

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch out app utilities with AragonUI utilities #1166

Switch out app utilities with AragonUI utilities #1166

ECWireless commented Jun 15, 2020 •

edited

Loading

ECWireless commented Jun 18, 2020

sohkai commented Jun 19, 2020

sohkai left a comment

sohkai Jun 19, 2020

sohkai Jun 19, 2020

bpierre Jul 1, 2020

Evalir left a comment •

edited

Loading

coveralls commented Jun 26, 2020 •

edited

Loading

bpierre Jun 19, 2020

bpierre Jul 1, 2020

		@@ -1,6 +1,6 @@
		import Aragon, { events } from '@aragon/api'
		import { addressesEqual } from '@aragon/ui'

	import { addressesEqual} from '@aragon/ui'
	import { addressesEqual } from '@aragon/ui'

Switch out app utilities with AragonUI utilities #1166

Switch out app utilities with AragonUI utilities #1166

Conversation

ECWireless commented Jun 15, 2020 • edited Loading

ECWireless commented Jun 18, 2020

sohkai commented Jun 19, 2020

sohkai left a comment

Choose a reason for hiding this comment

sohkai Jun 19, 2020

Choose a reason for hiding this comment

sohkai Jun 19, 2020

Choose a reason for hiding this comment

bpierre Jul 1, 2020

Choose a reason for hiding this comment

Evalir left a comment • edited Loading

Choose a reason for hiding this comment

coveralls commented Jun 26, 2020 • edited Loading

bpierre Jun 19, 2020

Choose a reason for hiding this comment

bpierre Jul 1, 2020

Choose a reason for hiding this comment

ECWireless commented Jun 15, 2020 •

edited

Loading

Evalir left a comment •

edited

Loading

coveralls commented Jun 26, 2020 •

edited

Loading