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

as a end user I would like to be able to attach ETH/tokens to my status update on gitcoin, so i can share money #5884

Merged
merged 41 commits into from
May 27, 2020

Conversation

zoek1
Copy link
Contributor

@zoek1 zoek1 commented Jan 25, 2020

Description
  • a 'attach tokens' button on the status update bottom

  • when its clicked, a user is prompted to select a token (any token listed at /settings/tokens can be added) and enter an amount
    image

  • user can submit the status update and when they do this function ( https://github.com/gitcoinco/web/blob/master/app/assets/v2/js/pages/bulk_payout.js#L140-L155 ) is called and they are prompted to send tokens. once they send tokens, the status update is posted
    DeepinScreenshot_20200131071339

  • on the status update, there is a nice visual box that shows there are tokens attached to the post.

  • after users comment on the post, the poster (and only the poster) can click an "award" button next to each comment to award any commenter the tokens.
    image

Refers/Fixes

#5881

Testing

@zoek1
Copy link
Contributor Author

zoek1 commented Jan 25, 2020

I'm based on changes of #5883, is the UI fine to specify the amount and award the tokens?

@zoek1 zoek1 force-pushed the feature/tip-comments branch from f8b8c0a to bcd11de Compare January 25, 2020 12:33
@codecov
Copy link

codecov bot commented Jan 25, 2020

Codecov Report

Merging #5884 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5884   +/-   ##
=======================================
  Coverage   26.70%   26.70%           
=======================================
  Files         293      293           
  Lines       27675    27675           
  Branches     4086     4086           
=======================================
  Hits         7391     7391           
  Misses      20019    20019           
  Partials      265      265           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70098d9...70098d9. Read the comment docs.

@danlipert
Copy link
Contributor

Excited for this! Let me know once its ready for review

@zoek1
Copy link
Contributor Author

zoek1 commented Jan 28, 2020

I'm asking to solve some questions in the original issue to unlock this 😃

@owocki
Copy link
Contributor

owocki commented Jan 31, 2020

I'm asking to solve some questions in the original issue to unlock this 😃

i wanna push on this soon.. r u still blocked? link me the questions?

@zoek1
Copy link
Contributor Author

zoek1 commented Jan 31, 2020

No problem @owocki, in working on this right now! In the morning the PR will ve ready to Review :thumbs_up:

@zoek1
Copy link
Contributor Author

zoek1 commented Jan 31, 2020

It's ready for review @owocki @danlipert 😃

@owocki
Copy link
Contributor

owocki commented Jan 31, 2020

feedback -

  1. wow it works end to end.. yay!
  2. https://bits.owocki.com/geurNd5Y can we make this area black, and also show the token amount + token name eg "0.01 ETH attached" isntead of' Tip attached"
  3. when i award the tip, does the to user get an email?
  4. when i visit the page incognito i get a 500 https://bits.owocki.com/qGuDm7PO
  5. when i post a new comment i get this issue https://bits.owocki.com/yAuLEK8J

@zoek1
Copy link
Contributor Author

zoek1 commented Feb 1, 2020

https://bits.owocki.com/geurNd5Y can we make this area black

i'm not sure what region should be black, The button, the comments or the status: 😅

@owocki

@zoek1
Copy link
Contributor Author

zoek1 commented Feb 5, 2020

@owocki fix conflicts done!

@zoek1
Copy link
Contributor Author

zoek1 commented Feb 7, 2020

@owocki I recently read the tip functionality is live, is this PR still relevant?

@octavioamu
Copy link
Contributor

I will say yes, still totally valid and interesting feature.

@owocki
Copy link
Contributor

owocki commented Feb 11, 2020

yeah staking on posts is an important mechanism.. would love to get this working on the new townsquare + get it into an upcoming release...

@zoek1
Copy link
Contributor Author

zoek1 commented Feb 14, 2020

Do you consider change something based on what you tested? I'll integrate this weekend the new square town into this.
@owocki

@zoek1 zoek1 force-pushed the feature/tip-comments branch from a24f38f to 0fc7a56 Compare February 18, 2020 06:10
@zoek1 zoek1 changed the base branch from master to stable February 18, 2020 06:11
@danlipert
Copy link
Contributor

Hey @zoek1 - @owocki is Out-of-office for a bit so I'll take over in the meantime, looks like this is rebased and ready for review and merge, right?

@zoek1 zoek1 force-pushed the feature/tip-comments branch from 6bffa80 to 716bc0e Compare May 5, 2020 17:32
<select class="form-control" id="attachToken">
<option value="0x0">ETH</option>
{% for token in TOKENS %}
<option value="{{ token.token_addres }}">{{ token.token_name }}</option>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this only tested with ETH? There is a typo here for the token_address

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I missed that. I tested with some ERC20 tokens like MANA and GNT.
image

@danlipert danlipert marked this pull request as draft May 13, 2020 15:34
@zoek1 zoek1 marked this pull request as ready for review May 14, 2020 21:54
@zoek1 zoek1 requested a review from danlipert May 15, 2020 02:41
@zoek1
Copy link
Contributor Author

zoek1 commented May 27, 2020

@danlipert do you think this could be merge soon? it's tested with eth and tokens 😛

@owocki

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.

6 participants