Issue #2508 Build - Front End of 'No Bounties' Placeholder on Gitcoin… #2530
Conversation
app/dashboard/models.py
Outdated
| @@ -2237,7 +2237,8 @@ def to_dict(self, activities=True, leaderboards=True, network=None, tips=True): | |||
| 'works_with_collected': works_with_collected, | |||
| 'works_with_funded': works_with_funded, | |||
| 'funded_bounties_count': funded_bounties.count(), | |||
| 'activities': [{'title': _('No data available.')}], | |||
| #'activities': [{'title': _('No data available.')}], | |||
There was a problem hiding this comment.
E265 block comment should start with '# '
Codecov Report
@@ Coverage Diff @@
## master #2530 +/- ##
=======================================
Coverage 29.87% 29.87%
=======================================
Files 162 162
Lines 12969 12969
Branches 1738 1738
=======================================
Hits 3874 3874
Misses 8971 8971
Partials 124 124
Continue to review full report at Codecov.
|
|
@thelostone-mc Could you help check? Thanks! 🙂 |
| </div> | ||
| {% endfor %} | ||
| {% else %} | ||
| <BR> |
There was a problem hiding this comment.
Can you change all occurrences into lowercase <br> ?
| </div> | ||
| {% endfor %} | ||
| {% else %} | ||
| <BR> | ||
| <div class="row" style="justify-content: center"><img width="240px" height="225px" src="http://kuhnchris.eu:8000/static/v2/images/tree.png" /></div><BR> |
There was a problem hiding this comment.
kuhnchris.eu advertising? 😄
Please use {% static "v2/images... like below 👍 .
There was a problem hiding this comment.
Hahaha weird, guess that slipped by me, sorry. ;-) Will change
| <div class="description" style="color: #666666; text-align: center">Start working on exciting bounties with the Issue Explorer</div> | ||
| <a class="btn btn-xs btn-success pulseClick" href="{% url "explorer" %}" style="background-color: #3E00FF; color: #FFFFFF; margin: 1em 0em 1em 0em; font-size: 1em ">Go to Issue Explorer</a> | ||
| </div> | ||
| <div style="width: 25%;justify-content: center;margin-right: 1em; padding: 1em; margin-top: 1em;" class="card col-6 col-lg-3"> |
There was a problem hiding this comment.
Can you try to use our existing CSS rules where possible? (app/assets/v2/css)
We also include bootstrap.css, which means we have something like m-{1,2..} for margin and 'p-{1,2,3..}` for padding etc.
👍
There was a problem hiding this comment.
If we have several predefined classes for margin or padding then, sure, I'll change that. Want me to add the other stuff in custom classes in assets/v2/css or just trying to reduce the amount of style use as much as possible?
There was a problem hiding this comment.
Just trying to reduce the use of the style attribute ;).
If the existing CSS rules doesn't fit your needs, you can of course use the style attribute or add it to the existing profile.css 👍
| </div> | ||
| <div style="width: 25%;justify-content: center;margin-right: 1em; padding: 1em; margin-top: 1em;" class="card col-6 col-lg-3"> | ||
| <img style="margin: 1em;" src="{% static "v2/images/Leaderboard.svg" %}"/> | ||
| <div class="description" style="color: #666666; text-align: center">Looking for awesome contributors to work on your project?</div> |
There was a problem hiding this comment.
And for INTL reasons we wrap text nodes with {% trans 'Looking for awesome contributors...' %} .
|
Thanks @pinkiebell for your review - please, if you got some more spare minutes, check my changes again if those changes work better, I did leave in the width: 25%, but everything else I think I solved via the bootstrap classes. |
| @@ -0,0 +1,48 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
We tend to svgo'ing ( https://github.com/svg/svgo ) these files.
👍
| @@ -8,6 +8,7 @@ | |||
| <link rel="stylesheet" href={% static "v2/css/profile.css" %}> | |||
| <link rel="stylesheet" href={% static "v2/css/tag.css" %}> | |||
| <link rel="stylesheet" href={% static "v2/css/tabs.css" %}> | |||
| <link rel="stylesheet" href={% static "v2/css/noprofile.css" %}> | |||
| <div class="row justify-content-center"><img width="240px" height="225px" src="{% static "v2/images/tree.png" %}"/></div><br> | ||
| <div class="text-center" style="color: #666666; font-size: 16px">{% trans 'Oops! Looks like this user has not participated in any bounty yet.<br>In the meantime, here are some things you can do:' %}</div><br> | ||
| <div class="justify-content-center row"> | ||
| <div style="width: 25%" class="card col-6 col-lg-3 mt-1 mr-1 p-1 justify-content-center"> |
There was a problem hiding this comment.
Almost done 😄
The indentation needs to be fixed,
we have a guide for reference: https://docs.gitcoin.co/mk_contributors/ .
I tested that locally and the footer is broken on views with activities :
| <div style="width: 25%" class="card col-6 col-lg-3 mt-1 mr-1 p-1 justify-content-center"> | ||
|
|
||
| <img class="m-1" src="{% static "v2/images/IssueExplorer.svg" %}"/> | ||
| <div class="description text-center" style="color: #666666">{% trans 'Start working on exciting bounties with the Issue Explorer' %}</div> |
There was a problem hiding this comment.
Can you push the {% trans.. on its own line like
<div .....>
{% trans '.....' %}
</div>
| {% else %} | ||
| <br> | ||
| <div class="row justify-content-center"><img width="240px" height="225px" src="{% static "v2/images/tree.png" %}"/></div><br> | ||
| <div class="text-center" style="color: #666666; font-size: 16px">{% trans 'Oops! Looks like this user has not participated in any bounty yet.<br>In the meantime, here are some things you can do:' %}</div><br> |
There was a problem hiding this comment.
We probably should avoid having html tags inside trans blocks.
<div ....>
{% trans '.....' %}
<br>
{% trans '.......' %}
</div>
|
Once again - thanks for checking. I cleaned up the HTML a bit more, and tried to use the margin-top: auto to push the buttons to the bottoms. SVGO'd the assets aswell. Also fixed the issue for the activity pages being displayed wrong. Thanks, |
thelostone-mc
added
enhancement
| </div> | ||
| </div> | ||
| {% else %} | ||
| <br> |
There was a problem hiding this comment.
Do we need a break ? Can we use mt-2 to the below div ?
There was a problem hiding this comment.
we could use a mt-2. I just saw a BR somewhere further above, hence I tried it that way, but, sure, we can go with margins if you prefer them generally.
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Could we fix up the indentation ?
| </a> | ||
| </div> | ||
| </div> | ||
| <div style="width: 25%" class="card col-6 col-lg-3 mt-1 mr-1 p-1 justify-content-center"> |
There was a problem hiding this comment.
Can we do it without width : 25% ? The col should suffice right ?
There was a problem hiding this comment.
the col would actually cause a 1/3 breakup instead of (empty) card card card (empty). But I could try to create 2 "empty" cols to have a 12.5/25/25/25/12.5 split.
| {% trans 'In the meantime, here are some things you can do:' %} | ||
| </div><br> | ||
| <div class="justify-content-center row"> | ||
| <div style="width: 25%" class="card col-6 col-lg-3 mt-1 mr-1 p-1 justify-content-center"> |
There was a problem hiding this comment.
Can we do it without width : 25% ? The col should suffice right ?
No problem, that's review not a hassle. 👍 |
|
Thanks! |
|
@kuhnchris left comments there! if you do that , I'll do the final refactoring needed and we can get this in |
|
Sweet, thanks. I'll implement the changes after returning home and notify if it's ready for your part! All the best, |
|
OK @thelostone-mc , please check once more. If the profile.html now is completly bonkers we might aswell just revert to the original version and simply put the new template in the else-branch, if that makes things easier. Thanks, |
|
Yes please ^_^ |
|
@thelostone-mc reverted profile.html to recent master; cleaned up - please check |
|
@willsputra this is what we've got at this moment
|
|
Looks good to me! |
|
code and screenshots LGTM... @kuhnchris i only see partial mobile screenshots tho.. does it look completely good on mobile? does it play nice with the kudos profile changes? |
|
F-ing sweet, but something fucked the SVGs up. :-( |
|
just redeployed and that seems to have helped |
|
Thanks @kuhnchris :D |
|
Thanks a lot @kuhnchris ! :) |
| @@ -19,7 +19,7 @@ | |||
| <div class="row"> | |||
| <div class="col-12 col-lg-4 profile-header__main-infos"> | |||
| <div class="clearfix"> | |||
| <img class="profile-header__avatar mb-1 mr-4" src="{% if profile.avatar and profile.avatar.avatar_url %}{{ profile.avatar.avatar_url }}{% else %}{{ profile.avatar_url }}{% endif %}"> | |||
| <img class="profile-header__avatar mb-1 mr-4" src="{% if profile.avatar and profile.avatar.avatar_url %}{{ profile.avatar.avatar_url }}{% else %}{{ profile.avatar_url }}{% endif %}" /> | |||
There was a problem hiding this comment.
@kuhnchris img tags don't require closure xD
There was a problem hiding this comment.
it depends, are we "just" HTML or XHTML? ;-)
But basically - you are correct.
https://www.w3.org/TR/html5/syntax.html#void-elements
| <link rel="stylesheet" href="{% static "v2/css/kudos/styles.css" %}" /> | ||
| <link rel="stylesheet" href={% static "v2/css/tabs.css" %}> | ||
| <link rel="stylesheet" href={% static "v2/css/tabs.css" %} /> |
There was a problem hiding this comment.
No syntactical reasoning for adding closures to link
There was a problem hiding this comment.
Same here for "void elements". I always thought every element needs closure, but apparently, not... My bad, sorry.
|
Thanks for the fix @kuhnchris Nit: There is no reason to modify line history for adding closures to |
|
Pardon! I will refrain from "fixing" HTML from now on. ;-) @mbeacom |
|
😂 - I'm not suggesting that you don't PR fixes for bugs and syntax refinements. Please don't take it that way... I'm merely saying that we shouldn't migrate void elements to use closures, since it's not syntactically necessary outside of XHTML. |
|
Don't worry, I can take it with a grain of salt. 👍 |
… Profile Page
Description
Implements source tree and link incase profile page is empty.
Fixes: #2508
Checklist
Affected core subsystem(s)
web
Testing
See issue thread above for in-progress and final screenshot(s)!
Refers/Fixes
Fixes: #2508