Make It Clear Issue Has Been Crowdfunded V2 #2173

pinkiebell · 2018-09-06T22:11:44Z

app/assets/v2/js/pages/dashboard.js

stickler-ci · 2018-09-06T22:12:14Z

app/assets/v2/js/shared.js


-    result['p'] = ((result['experience_level'] ? result['experience_level'] : 'Unknown Experience Level') + ' &bull; ');
+      if (tokens.length) {
+        var obj = {};


Expected blank line after variable declarations. (newline-after-var)

stickler-ci · 2018-09-06T22:12:14Z

app/assets/v2/js/shared.js


-    if (result['status'] === 'done')
+        while (tokens.length) {
+          var tokenName = tokens.shift();


Expected blank line after variable declarations. (newline-after-var)

stickler-ci · 2018-09-06T22:12:14Z

app/assets/v2/js/shared.js

-      var opened_when = timeDifference(new Date(), new Date(result['web3_created']), true);
+      var openedWhen = timeDifference(dateNow, new Date(result['web3_created']), true);
+      var timeLeft = timeDifference(dateNow, dateExpires);
+      var expiredExpires = new Date() < dateExpires ?   'Expires' : 'Expired';


Multiple spaces found before ''Expires''. (no-multi-spaces)

stickler-ci · 2018-09-06T22:12:15Z

app/assets/v2/js/shared.js


-      result['p'] += ('Opened ' + opened_when + ' ago, Expires in ' + timeLeft);
+      result['p'] += ('Opened ' + openedWhen + ' ago, ' + expiredExpires   + ' ' + timeLeft);


Multiple spaces found before '+'. (no-multi-spaces)

stickler-ci · 2018-09-06T22:12:15Z

app/assets/v2/js/shared.js

+
+    if (renderForExplorer) {
+      if (typeof web3 != 'undefined' && web3.eth.coinbase == result['bounty_  owner_address']) {
+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">mine</span></a>';


Expected indentation of 8 spaces but found 10. (indent)

stickler-ci · 2018-09-06T22:12:15Z

app/assets/v2/js/shared.js

+    if (renderForExplorer) {
+      if (typeof web3 != 'undefined' && web3.eth.coinbase == result['bounty_  owner_address']) {
+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">mine</span></a>';
+        } else if (result['fulfiller_address'] !== '0x000000000000000000000000  0000000000000000') {


Expected indentation of 6 spaces but found 8. (indent)

stickler-ci · 2018-09-06T22:12:15Z

app/assets/v2/js/shared.js

+      if (typeof web3 != 'undefined' && web3.eth.coinbase == result['bounty_  owner_address']) {
+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">mine</span></a>';
+        } else if (result['fulfiller_address'] !== '0x000000000000000000000000  0000000000000000') {
+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">' + result['status'] + '</span></a>';


Expected indentation of 8 spaces but found 10. (indent)

stickler-ci · 2018-09-06T22:12:15Z

app/assets/v2/js/shared.js

+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">mine</span></a>';
+        } else if (result['fulfiller_address'] !== '0x000000000000000000000000  0000000000000000') {
+          result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outli  ne-dark" role="button" href="#">' + result['status'] + '</span></a>';
+        }


Expected indentation of 6 spaces but found 8. (indent)

codecov · 2018-09-06T22:23:54Z

Codecov Report

Merging #2173 into master will decrease coverage by 0.02%.
The diff coverage is 18.51%.

@@            Coverage Diff             @@
##           master    #2173      +/-   ##
==========================================
- Coverage   27.75%   27.72%   -0.03%     
==========================================
  Files         142      142              
  Lines       11506    11519      +13     
  Branches     1562     1563       +1     
==========================================
+ Hits         3193     3194       +1     
- Misses       8201     8213      +12     
  Partials      112      112

Impacted Files	Coverage Δ
app/dashboard/models.py	`49.73% <18.51%> (-0.42%)`	⬇️

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 aa161c8...68d0f7e. Read the comment docs.

thelostone-mc · 2018-09-07T08:32:23Z

app/assets/v2/js/shared.js

@@ -955,63 +955,107 @@ var usdToAmount = function(event) {
  });
 };

-function renderBountyRowsFromResults(results) {
+function renderBountyRowsFromResults(results, renderForExplorer) {


thelostone-mc · 2018-09-07T08:36:39Z

app/assets/v2/images/coin.svg

+            </g>
+        </g>
+    </g>
+</svg>


@pinkiebell can we run svgo against this file ?

pinkiebell · 2018-09-07T09:12:22Z

@thelostone-mc svgo-ing Done 👍

pinkiebell · 2018-09-07T09:17:24Z

Side note: I labelled this pr as wip because of one open question:

How do we handle tokens with no usd conversion?
Right now we have no indicator for that and fixing requires backend changes,
do we want to handle that case in this PR or defer?

PixelantDesign · 2018-09-07T11:06:00Z

@pinkiebell could we display the token with no usd conversion in the tool tip but not average in the final USD value?

stickler-ci · 2018-09-07T22:54:12Z

app/assets/v2/js/pages/bounty_details.js

+    let leftHtml = '';
+    let rightHtml = '';
+
+    leftHtml +=


'+=' should be placed at the beginning of the line. (operator-linebreak)

stickler-ci · 2018-09-07T22:54:12Z

app/assets/v2/js/pages/bounty_details.js

+    leftHtml +=
+      '<p class="m-0">&nbsp;&nbsp;&nbsp;' + bountyTokenAmount + ' ' + bountyTokenName + '</p>';
+
+    rightHtml +=


'+=' should be placed at the beginning of the line. (operator-linebreak)

stickler-ci · 2018-09-07T22:54:12Z

app/assets/v2/js/pages/bounty_details.js

+      const tokenName = tokens.shift();
+      const obj = val[tokenName];
+      const ratio = obj['ratio'];
+      const amount = obj['amount']


Missing semicolon. (semi)

stickler-ci · 2018-09-07T22:54:12Z

app/assets/v2/js/pages/bounty_details.js

+      const timePeg = timeDifference(dateNow, new Date(obj['timestamp']), false, 60 * 60);
+      const tooltip = '$' + normalizeAmount(usd, decimals) + ' ' + tokenName + ' in crowdfunding';
+
+      leftHtml +=


'+=' should be placed at the beginning of the line. (operator-linebreak)

stickler-ci · 2018-09-07T22:54:12Z

app/assets/v2/js/pages/bounty_details.js

+
+      leftHtml +=
+        '<p class="m-0">+ ' + funding + ' ' + tokenName + '</p>';
+      rightHtml +=


'+=' should be placed at the beginning of the line. (operator-linebreak)

pinkiebell · 2018-09-07T23:05:35Z

The only thing what's left is the caret(arrow) with the right color(same as link color).
I'm also open for another round of improvements 👍

thelostone-mc · 2018-09-10T06:42:25Z

@pinkiebell could you rebase it once ? we'll put this up on staging and test it out there today

pinkiebell · 2018-09-10T09:41:22Z

@thelostone-mc Done.

~~Errors still in this PR are likely:~~
~~The tooltip misses the 'as of' .~~
~~I'm a little bit confused with the '$' sign in the crowdfunding tooltip,~~
~~because I always think of dollar valuation if I see the '$', was that actually right or is this wrong to~~
~~convert the crowdfunding token value to dollar in the tooltip?~~

stickler-ci · 2018-09-10T15:01:03Z

app/assets/v2/js/shared.js

+
+  ele.className = 'tag token';
+  span.innerHTML = amount + ' ' + tokenName +
+    (isCrowdfunded ? '<i class="fas fa-users ml-1"></i>' : '' );


There should be no spaces inside this paren. (space-in-parens)

pinkiebell · 2018-09-18T13:37:01Z

@thelostone-mc
Any news?

SaptakS · 2018-09-18T16:09:32Z

@pinkiebell I will be reviewing it in some time and get back to you. :)

SaptakS · 2018-09-24T14:42:01Z

@pinkiebell I am seeing this in every non-crowfunded bounty details page. Can you please check?

mbeacom · 2018-09-17T17:05:10Z

app/dashboard/models.py

-            return ""
+        tokens = afs.keys()
+
+        if len(tokens) == 0:


No need to check len() here. Simply if not tokens:

mbeacom · 2018-09-17T17:14:44Z

app/dashboard/models.py

-            return_dict['usd_value'] += tip.value_in_usdt if tip.value_in_usdt else 0
-        return return_dict
+            token = tip.tokenName
+            obj = ret.get(token, None)


Can you default to {}:

obj = ret.get(token, {}) if not obj: obj['amount'] = 0.0

pinkiebell · 2018-09-24T20:16:39Z

@SaptakS
I can't reproduce this behaviour, what wonders me is the 'as ofnow' in the USD tag.
That looks like the JS code is not from this PR. Maybe a cache issue?

@mbeacom
I addressed your comments. I did a rebase though, hope that's ok.

pinkiebell · 2018-09-24T20:42:39Z

@SaptakS
Fun times 😄 . This may be problem with docker and inotify events.
Just reproduced that in the issue explorer because I switched branches/rebased and the django app didn't reload (fs inotify events).

SaptakS · 2018-09-24T21:51:23Z

@pinkiebell The icon beside See All is opposite according to me. It should point downwards when it is closed and upwards when it is open. cc: @thelostone-mc @mbeacom @PixelantDesign

SaptakS · 2018-09-24T21:52:53Z

app/assets/v2/js/pages/bounty_details.js


-        tooltip_info.push(template);
-      }
+    const decimals = 2;


We are showing 3 decimal places in almost all places. Any reason why we are showing only 2 for crowdfunded amount?

@SaptakS Aesthetics? 😄 Fixed!

SaptakS · 2018-09-24T22:23:44Z

LGTM apart from above comments.

pinkiebell · 2018-09-25T09:14:21Z

@PixelantDesign

That's the ETH amount in USD, we should probably change the formatting?
Did you mean putting the green USD tag on the left or should they all be aligned to the right for example? That could be a little bit tricky with our current CSS if we want to make sure that they are all aligned top-down.

@mbeacom
Hehe, we can tell future prices, nice! I will put a fix in 👍

pinkiebell · 2018-09-25T23:35:46Z

pinkiebell · 2018-09-26T22:04:30Z

@mbeacon @SaptakS @PixelantDesign
FInal review? 🙏

thelostone-mc · 2018-09-30T20:59:26Z

@pinkiebell one final rebase and then we can throw it on staging

Fixes #2164

Also revamped the additional_funding_summary bounty-property

pinkiebell · 2018-10-01T09:03:27Z

@thelostone-mc
Done 👍

PixelantDesign · 2018-10-01T15:31:02Z

@pinkiebell Looking good to me...One thing I see...

It should not be possible to have 791 cents.

And here

pinkiebell · 2018-10-01T16:46:26Z

@PixelantDesign
Resolved 👍 . We now display token amounts with max .3 decimals and dollar conversion .2 decimals.

thelostone-mc

🚢

stickler-ci reviewed Sep 6, 2018

View reviewed changes

thelostone-mc reviewed Sep 7, 2018

View reviewed changes

thelostone-mc requested review from thelostone-mc and SaptakS September 7, 2018 08:34

thelostone-mc reviewed Sep 7, 2018

View reviewed changes

mbeacom previously approved these changes Sep 7, 2018

View reviewed changes

pinkiebell dismissed mbeacom’s stale review via ce41715 September 7, 2018 22:53

stickler-ci reviewed Sep 7, 2018

View reviewed changes

pinkiebell changed the title ~~WIP: Make It Clear Issue Has Been Crowdfunded V2~~ Make It Clear Issue Has Been Crowdfunded V2 Sep 10, 2018

stickler-ci reviewed Sep 10, 2018

View reviewed changes

mbeacom added needs review frontend This needs frontend expertise. labels Sep 12, 2018

owocki mentioned this pull request Sep 17, 2018

Crowdfund contributions not being included in some bounty totals on issue explorer #2228

Closed

mbeacom reviewed Sep 24, 2018

View reviewed changes

SaptakS reviewed Sep 24, 2018

View reviewed changes

pinkiebell dismissed SaptakS’s stale review via 4c33dd0 September 25, 2018 10:21

pinkiebell and others added 16 commits October 1, 2018 10:59

WIP: Make It Clear Issue Has Been Crowdfunded V2

ebdddbd

Fixes #2164

svgo coin.svg

61e6dff

crowdfunding: some ui updates.

54d6d73

Also revamped the additional_funding_summary bounty-property

crowdfunding: small adjustments

8d7ef4d

crowdfunding: fix-isort

ba74a03

crowdfunding: final bits

3daef40

crowdfund: stylefix again

a359f11

crowdfund: lint

577992c

crowdfunding: style fixes

3505caa

crowdfunding: responsive fix

9f57080

crowdfunding: decimal places 2 -> 3

cefd466

crowdfunding: fix accordion rotation

067f646

crowdfunding: another round of improvements

d0d44e1

crowdfunding: ditto

9befa27

crowdfunding: hiring logo and layout

14bda0d

crowdfunding: hiring logo

fe69a72

crowdfunding: normalizeAmount - decimals for tokens = 3 and dollar = 2

68d0f7e

thelostone-mc approved these changes Oct 3, 2018

View reviewed changes

thelostone-mc merged commit e349ace into gitcoinco:master Oct 3, 2018

pinkiebell deleted the crowdfund branch November 6, 2018 08:41


		result['p'] += ('Opened ' + opened_when + ' ago, Expires in ' + timeLeft);
		result['p'] += ('Opened ' + openedWhen + ' ago, ' + expiredExpires + ' ' + timeLeft);

Make It Clear Issue Has Been Crowdfunded V2 #2173

Make It Clear Issue Has Been Crowdfunded V2 #2173

Conversation

pinkiebell commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 6, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinkiebell commented Sep 7, 2018

pinkiebell commented Sep 7, 2018

PixelantDesign commented Sep 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinkiebell commented Sep 7, 2018

thelostone-mc commented Sep 10, 2018

pinkiebell commented Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

pinkiebell commented Sep 18, 2018

SaptakS commented Sep 18, 2018

SaptakS commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinkiebell commented Sep 24, 2018

pinkiebell commented Sep 24, 2018

SaptakS commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaptakS commented Sep 24, 2018

pinkiebell commented Sep 25, 2018

pinkiebell commented Sep 25, 2018 • edited Loading

pinkiebell commented Sep 26, 2018

thelostone-mc commented Sep 30, 2018

pinkiebell commented Oct 1, 2018

PixelantDesign commented Oct 1, 2018 • edited by thelostone-mc Loading

pinkiebell commented Oct 1, 2018

thelostone-mc left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 6, 2018 •

edited

Loading

pinkiebell commented Sep 10, 2018 •

edited

Loading

pinkiebell commented Sep 25, 2018 •

edited

Loading

PixelantDesign commented Oct 1, 2018 •

edited by thelostone-mc

Loading