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

Voting new styles for 0.8 #935

Merged
merged 176 commits into from Aug 19, 2019

Conversation

@AquiGorka
Copy link
Member

commented Jul 26, 2019

Fixes #714.

How to test

Either ask the author if the latest changes have been deployed to preview.aragon.org

or

Assuming everything is in the ~/projects directory (replace by your own).

First time

Clone this repo + Aragon client’s + aragonUI’s if not done already:

cd ~/projects
git clone git@github.com:aragon/aragon-apps.git
git clone git@github.com:aragon/aragon.git
git clone git@github.com:aragon/aragon-ui.git

Create a link for aragonUI (npm might tell you that it is already done):

cd ~/projects/aragon-ui
npm link

Getting updates

Here we are updating the branches + reinstalling the dependencies + linking aragonUI again.

cd ~/projects/aragon-ui
git checkout newstyle
git pull origin newstyle
npm install

cd ~/projects/aragon
git checkout newstyle
git pull origin newstyle
npm install
npm link @aragon/ui

cd ~/projects/aragon-apps/apps/token-manager/app
git checkout newstyle/voting
git pull origin newstyle/voting
npm install
npm link @aragon/ui

Running it

We can now start the two servers (the app + Aragon client). In one shell session (or tab in your terminal app):

cd ~/projects/aragon-apps/apps/token-manager/app
npm start

In another session (we are asking the client to load apps running locally):

cd ~/projects/aragon
env REACT_APP_ASSET_BRIDGE=local yarn start

Open http://localhost:3000/ in your browser.

@coveralls

This comment has been minimized.

Copy link

commented Jul 26, 2019

Coverage Status

Coverage decreased (-0.3%) to 97.635% when pulling 4eb8404 on newstyle/voting into a4f873e on newstyle-0.8.

@AquiGorka AquiGorka force-pushed the newstyle/voting branch from 4a2b7cd to ddbea7b Aug 16, 2019

AquiGorka and others added some commits Aug 16, 2019

Voting: don't autolink text in cards
Clicking on the link can be confusing with wanting to actually open the
card itself, rather than the link.
@@ -39,16 +38,24 @@ const VoteActions = React.memo(({ vote, onVoteYes, onVoteNo, onExecute }) => {
const hasVoted = [VOTE_YEA, VOTE_NAY].includes(connectedAccountVote)

useEffect(() => {
let cancelled = false

This comment has been minimized.

Copy link
@AquiGorka

AquiGorka Aug 19, 2019

Author Member

Hmm, for this you could've used the usePromise hook that the inner functions rely on.

This comment has been minimized.

Copy link
@sohkai

sohkai Aug 19, 2019

Member

Hmm yes, but usePromise()'s API is based on returning the resulting value from the promise (so you can use it) whereas we just want to cancel it here and avoid a side effect.

This comment has been minimized.

Copy link
@AquiGorka

AquiGorka Aug 19, 2019

Author Member

Pseudo code here (this is what I was thinking):

const promise = Promise.all([canUserVotePromise, canExecutePromise, userBalancePromise])
const value = usePromise(promise, false)
setReady(value)

But yeah, there are other things that might need to be worked on.

This comment has been minimized.

Copy link
@sohkai

sohkai Aug 19, 2019

Member

Yeah, I'm worried there might be funny things with how the state gets updated / triggered (compared to now, which is relatively obvious what happens on cancellation / updates), and to be honest usePromise() is something we should find a better abstraction for since it's kind of clunky.

sohkai added some commits Aug 19, 2019

@sohkai sohkai merged commit 3ee37de into newstyle-0.8 Aug 19, 2019

3 of 6 checks passed

Travis CI - Pull Request Build Errored
Details
Travis CI - Branch Build Failed
Details
License Compliance FOSSA is analyzing this commit
Details
WIP Ready for review
Details
coverage/coveralls Coverage remained the same at 98.488%
Details
license/cla Contributor License Agreement is signed.
Details

@sohkai sohkai deleted the newstyle/voting branch Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.