Feature/new look grants #7332

zoek1 · 2020-09-04T11:30:11Z

Description

star/unstar a grant using vue
list view of all the grants,
Fix grant nav sort issue

https://www.loom.com/share/8eec68f5f2d243c58968f9bd2db25a96

Refers/Fixes

#7163
#7166
#7190

Testing

zoek1 · 2020-09-04T13:21:35Z

@thelostone-mc are you have some time to check this please? 🙂

octavioamu

Well done Zoek, I left just some few comments

octavioamu · 2020-09-04T13:34:55Z

app/grants/templates/grants/shared/landing_grants.html

-                {% include 'grants/card/index.html' %}
-              </div>
-            {% endfor %}
+        <div class="infinite-container row" v-if="grants">


I think scroll method is not being call here so probably pagination will not work you need to use the directive scroll.passive (check notifications.html)

octavioamu · 2020-09-04T13:36:13Z

app/grants/templates/grants/index.html

-              <div class="col-12 col-lg-3 col-xl-2" id="sidebar_container">
-                {% include 'grants/shared/sidebar_search.html' %}
-              </div>
+            <div id="grants-showcase">


missing v-cloak to hide the vue components until data is loaded (avoid reflow)

octavioamu · 2020-09-04T13:36:32Z

app/grants/templates/grants/shared/landing_grants.html

-                <i class="fas fa-spinner fa-spin mr-2"></i>
+        </div>
+
+        <!--p class="mb-0 text-center py-4" v-if="has_next">


thelostone-mc · 2020-09-06T14:29:11Z

@zoek1 could you

point this to the integration branch https://github.com/gitcoinco/web/tree/round-7-integration
show how the page looks in list view when there is no active CLR
rebase with integration commits
ensure travis pases

thelostone-mc · 2020-09-06T14:33:56Z

app/grants/templates/grants/card/front.html

+            <i class="fa fa-star" ></i><span class="ml-1 font-body">Following</span>
+          {% endif %}
+        </button>
+        {% endif %}


^ indent it right

{% if profile.user.is_authenticated %} .... {% endif %}

Why are there changes in this file when this you've removed it's usage in
https://github.com/gitcoinco/web/pull/7332/files#diff-a9ea52761a5188515132b584f9014d16L9 ?

thelostone-mc · 2020-09-06T14:34:44Z

app/grants/templates/grants/components/card.html

+{% load i18n static humanize grants_extra %}
+<grant-card inline-template :grant="grant" :cred="credentials" :view="view">
+  <div class="grant-item grant-item--list flex-md-row flex-column" v-if="view == 'list'">
+      <div class="grant-item__img col-md-1 d-md-block d-none">


^ 1 tab= 2 spaces indentation please

thelostone-mc · 2020-09-06T14:36:00Z

app/grants/templates/grants/components/card.html

+            <div class="match__round pb-1 pt-2 mb-2">
+              <h2 class="title col-8 font-caption g-bold">
+                  CLR MATCH ROUND 6
+              </h2>


^ why is this hardcoded ?
This is something which should be available from the backend API.

thelostone-mc · 2020-09-06T14:38:16Z

app/grants/templates/grants/components/card.html

+
+          <div class="col-12 pb-3" v-if="grant.is_clr_eligible">
+            {# with short=True #}
+            {% include 'grants/card/clr_estimate.html'  %}


shouldn't this be {% include 'grants/card/clr_estimate.html' with short=True %}

thelostone-mc · 2020-09-06T14:38:19Z

app/grants/templates/grants/components/card.html

+    </div>
+  </a>
+    </div>
+</grant-card>


^ could you redo the text and the conditions based on what's in front.html
this code seems a lil outdated

thelostone-mc

@zoek1 looking into this code ! A lot of things would break.
Looks like you started with an outdated branch (over a month old) and never rebased after that

I'd recommend taking the latest code and porting your css + model changes
and redoing the views + HTML changes with the latest code and ensuring you don't overwrite anything

thelostone-mc · 2020-09-06T14:39:52Z

app/grants/templates/grants/detail/funding.html

      {% if not is_team_member %}
-        <div class="pt-2 mb-1">
-          <div class="text-right">
+      <div>


why the extra div ? + indent fix needed

thelostone-mc · 2020-09-06T14:40:27Z

app/grants/templates/grants/detail/funding.html

+                  <i class="far fa-star" ></i><span class="ml-1 font-body">Follow</span>
+                {% else %}
+                  <i class="fa fa-star" ></i><span class="ml-1 font-body">Following</span>
+                {% endif %}


^ can be taken outside the if check

thelostone-mc · 2020-09-06T14:40:39Z

app/grants/templates/grants/detail/funding.html

+                {% endif %}
+            </button>
+          {% else %}
+            <span></span>


Is this needed?

thelostone-mc · 2020-09-06T14:41:37Z

app/grants/templates/grants/index.html

+                  {% elif type == 'about' %}
+                  {% include 'grants/shared/landing_announce.html' %}
+                  {% elif type == 'stats' %}
+                  <div id="insert_iframe">


^ could we get this indented right

thelostone-mc · 2020-09-06T14:43:20Z

app/grants/templates/grants/index.html

+      document.current_type = "{{ type|default:'all' }}";
+      document.selected_category = "{{ selected_category }}"
+      document.keyword = "{{ keyword }}"
+      document.following = "{{ following }}"


this would pop up sentry bugs
Let's wrap em in if checks and keep ; consistent

thelostone-mc · 2020-09-06T14:56:05Z

app/grants/views.py

+    ).keyword(keyword)
+
+    _grants = _grants.order_by(sort, 'pk')
+    ____ = _grants.first()


what is this for ?

thelostone-mc · 2020-09-06T14:56:42Z

app/grants/views.py

+        _grants = _grants.active()
+
+    if grant_type != 'all':
+        _grants = _grants.filter(grant_type=grant_type)


I don't this would works!
It would be grant_type__name

thelostone-mc · 2020-09-06T14:58:16Z

app/grants/views.py

+        _grants = _grants.order_by(f"-{field_name}")
+
+    if category:
+        _grants = _grants.filter(Q(categories__category__icontains=category))


I don't think would work either as it's now from a table

thelostone-mc · 2020-09-06T14:58:29Z

app/grants/views.py

+        #        {'label': 'Health', 'keyword': 'health', 'count': health_grants_count},
+        {'label': 'Matic: Build-n-Earn', 'keyword': 'matic', 'count': matic_grants_count},
+        {'label': 'Crypto for Black Lives', 'keyword': 'change', 'count': change_count},
+    ]


thelostone-mc · 2020-09-06T15:02:57Z

app/grants/templates/grants/card/front.html

+            <i class="fa fa-star" ></i><span class="ml-1 font-body">Following</span>
+          {% endif %}
+        </button>
+        {% endif %}


Why are there changes in this file when this you've removed it's usage in
https://github.com/gitcoinco/web/pull/7332/files#diff-a9ea52761a5188515132b584f9014d16L9 ?

zoek1 added 4 commits August 18, 2020 00:46

Add star functionality on grants

8afe1b2

Remove immediately grants

4eb0b5a

migrate grant card to vue component

bb588b5

Implement list view

af957eb

zoek1 requested review from danlipert, frankchen07, octavioamu, PixelantDesign and thelostone-mc as code owners September 4, 2020 11:30

Merge branch 'master' into feature/new-look-grants

557afa6

octavioamu suggested changes Sep 4, 2020

View reviewed changes

thelostone-mc reviewed Sep 6, 2020

View reviewed changes

thelostone-mc requested changes Sep 6, 2020

View reviewed changes

zoek1 closed this Sep 7, 2020

zoek1 mentioned this pull request Sep 7, 2020

add list view for grants and migrate to vue #7336

Merged

thelostone-mc mentioned this pull request Sep 17, 2020

Implement track of grants #7245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/new look grants #7332

Feature/new look grants #7332

zoek1 commented Sep 4, 2020

zoek1 commented Sep 4, 2020

octavioamu left a comment

octavioamu Sep 4, 2020

octavioamu Sep 4, 2020

octavioamu Sep 4, 2020

thelostone-mc commented Sep 6, 2020 •

edited

Loading

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc left a comment

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

thelostone-mc Sep 6, 2020

Feature/new look grants #7332

Feature/new look grants #7332

Conversation

zoek1 commented Sep 4, 2020

Description

Refers/Fixes

Testing

zoek1 commented Sep 4, 2020

octavioamu left a 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

thelostone-mc commented Sep 6, 2020 • edited Loading

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

thelostone-mc left a 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

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

thelostone-mc commented Sep 6, 2020 •

edited

Loading