hackforla / website

Hack for LA's website
https://www.hackforla.org
GNU General Public License v2.0
325 stars 778 forks source link

Update home page project cards to prioritize by status #303

Closed harishlingam closed 4 years ago

harishlingam commented 4 years ago

Overview

Update home page project cards to prioritize by status

Action Items

Update home page project cards to prioritize by status: (1) Inactive projects should display lower on the page, whereas active ones should display earlier (2) Randomly sort projects.

Resources/Instructions

ExperimentsInHonesty commented 4 years ago

@KianBadie is recommending that we move the following code to its own include line 8-49 from this file: https://github.com/hackforla/website/blob/gh-pages/_includes/current-projects.html

<li class="project-card">
              <div class="project-card-inner">
                <div class="project-tmb">
                  <img src={{ item.image | absolute_url }} class="project-tmb-img" alt={{item.alt}}/>
                </div>
                <div class="project-body">
                  <div class='status-indicator {{ "status-" | append: item.status }}'>
                    <h5 class='status-text'>{{ item.status }}</h5>
                  </div>
                  <h4 class="project-title">{{ item.title }}</h4>
                  <p class="project-description">{{ item.description }}</p>
                  {%- if item.links -%}
                    <div class="project-links">
                      <strong>Links: </strong> 
                        {%- for item in item.links -%} 
                          <a href={{ item.url }} rel="noopener" target='_blank'> {{ item.name }}</a>{%- if forloop.last == false -%},
                          {%- endif -%}
                        {%- endfor -%}
                    </div>
                  {%- endif -%} 
                  {%- if item.partner -%}
                    <div class="project-partner">
                    <strong>Partner: </strong>
                      {{ item.partner }}
                    </div>
                  {%- endif -%}
                  {%- if item.looking -%}
                    <div class="project-needs">
                      <strong>Looking for: </strong>
                        {{ item.looking }}
                    </div>
                  {%- endif -%}

                  {%- if item.location -%}
                    <div class="project-location">
                      <strong>Location: </strong>
                        {{ item.location }}
                    </div>
                  {%- endif -%}
                </div>
              </div>
            </li>

then we would loop through project cards, looking for active and then calling the include, and then rebooting, etc.

ExperimentsInHonesty commented 4 years ago

@StephenVNelson @cnk Do either of you have any suggestions on any other ways we could sort the cards on first view on the hompage with active appearing at top. See previous comment for Kian's suggestion.

cnk commented 4 years ago

I like the suggestion of making the card itself a separate include. No matter how we go about sorting (multiple loops through site.projects or an inline sort), having the card's display logic in its own file will make the sorting logic easier to understand

joelparkerhenderson commented 4 years ago

My opinion about the active project card order: do it with randomization, either per build or per view. Randomization is the best way to be fair and balanced among all the active projects, and also gives us the best data about what's compelling to our visitors.

If the order isn't random, then I advocate for the order to be by quality impact, which to me means the projects that will help us the most with attracting clients, funders, volunteers, partners, etc.

joelparkerhenderson commented 4 years ago

I have code ready to show you all, on its own topic branch, when you have some kind way to push a branch to the repo. Thanks!

KianBadie commented 4 years ago

@ExperimentsInHonesty So I have an implementation of listing the project cards in the order that we want (without randomization. If that is going to be implemented let me know and we can figure out how to combine @joelparkerhenderson's and the feature of listing projects by status). It involved moving the project cards to their own includes files titled "project-card.html". What Kegan showed me last night at the hack night when we brought this issue to him was a way to make the project card even more modular. This was done by adding this line of code at the top of project-card.html:

{% assign project = include.project %}

And referencing each field of a project in the project-card.html using project.name or project.links. And when we want to include a project card while looping through all our project using some other variable in a forloop, we would call it like this:

        {%- for item in site.projects -%}
          {%- if item.hide != true and item.status == 'Active' -%}
            {%- include project-card.html project=item -%}
          {%- endif -%}
        {%- endfor -%}

Including the project card using project=item removes the requirement of project-card.html using the same variable as the forloop that is iterating through it (before, everything was referenced as item.name or item.links). This just reduces the possibility of errors and makes the code more reusable. So thanks @thekaveman!

Also, @thekaveman, I know that we talked about trying to use filters on the projects list in order to make the code cleaner/more readable. It turns out that you can't use a filter in a forloop, but you can only use a filter to create a new array. So that would be 4 extra lines of code and the same amount of forloops. In addition, checking the value of item.hide using a filter wasn't feasible because we only set the field as true if we want to hide it, but leave the field absent if it is to show. And I don't think filters can check for something like {% ... | where: 'hide', 'not true' %}. So the only way to check that is using a forloop and testing for item.hide != true. So the 2 options I could think of (unless there's some other way) would be a loop at the beginning of all of loops that check project status in order to create a new array of projects with hide != true, or checking for that value for each project we loop through (the way it is currently being done). I figured the former and the 4 extra lines from the filters were going to create more lines of code than save, and decided to leave the filters out and keep the loops themselves the way they were. Let me know if there's another way or desire for change, but I remember you saying this part wasn't a big deal either haha.

@ExperimentsInHonesty, would you like me to make a pull request? Or wait for more discussion?

thekaveman commented 4 years ago

@KianBadie

It turns out that you can't use a filter in a forloop, but you can only use a filter to create a new array.

That's a bummer, i forgot about that limitation. I think I didn't fully understand what we were talking about the other night (I didn't realize it was related to this issue), so I may have given poor advice. I see three approaches:

1. Filter for hide first, check status in each loop

This is what you suggested. You can test for nil (empty/false) using where

{% assign projects = site.projects | where "hide", nil %}
{% for project in projects %}
  {% if project.status = "Active" %}
  ... # render project card
  {% endif %}
{% endfor %}

2. Filter for status first, check hide in each loop

You can use the tag {% unless %} to mean if not

{% assign projects = site.projects | where "status", "Active" %}
{% for project in projects %}
  {% unless project.hide %}
  ... # render project card
  {% endunless %}
{% endfor %}

This could be further cleaned up with another include for project-list, like:

_includes/project-list.html

{% for project in include.projects %}
  {% unless project.hide %}
  ... # render project card
  {% endunless %}
{% endfor %}

Then in current-projects.html you could have (once for each status):

{% assign projects = site.projects | where "status", "Active" %}
{% include project-list.html projects=projects %}

3. Use JavaScript

Solutions 1 and 2 still don't solve for the randomization issue, which will only re-randomize on each build (once per push to gh-pages).

Another approach is to render the project data to a JSON file, and then use JavaScript to read that file and build up the lists randomly each time.

projects.json

---
---
{% site.projects | jsonify %}

The triple-dashes --- at the top tell Jekyll to process the JSON file as a Jekyll template

This would be a file with the projects in a JSON array of objects. Instead of rendering using Jekyll/Liquid templates, JavaScript would read that file on page load and reorder randomly for display. This is obviously more complicated than 1 and 2, and starts moving in the direction of JS frameworks (Angular, React, etc.)

joelparkerhenderson commented 4 years ago

Something like this JavaScript pseudocode?

let projects = fetch("projects.json").then(response => response.json());

let rank = { "Active": 1, "On Hold": 2, "Completed": 3 };

function cmp(a, b) {
  return a == b ? 0 : a < b ? -1 : 1;
}

function cmp_random() {
  return Math.random() < 0.5 ? - 1 : 1;
}

function cmp_status(a, b){ 
  return a.status == b.status ? cmp_random() : cmp(rank[a.status], rank[b.status]); 
}

let sorted = projects.sort(cmp_status);
KianBadie commented 4 years ago

@thekaveman I like the idea for having a separate includes project-list. That would clean up the current-projects.html even more. A question with comparing the 1st and 2nd solution. Are there any preferences? I feel like having one or the other or both solutions would both work out, so I'm not sure what to lean towards.

Also a question with the 3rd solution. So all the rendering would happen with Javascript right? Totally doable, but would definitely be a bigger structural change. I actually just did some research and it seems Liquid might have a way to randomly select items from an array (and hopefully also a collection). Stackoverflow question and see the Sample filter on this doc. Going to experiment with this today. If this works it might make the process a whole lot easier, since the structure of the includes file can remain familiar/simple and avoiding rendering with Javascript.

And @joelparkerhenderson, so that psuedocode would sort the projects both by status and randomly? If so, nice!

Update: Unfortunately, with my current research, it seems that the sample filter will not work. It looks like that array is randomly created on each website build, rather than each page visit.

ExperimentsInHonesty commented 4 years ago

Point of clarification: I appreciate @joelparkerhenderson ‘s suggestion about randomization, and we will consider that for another iteration along with sorting by projects that have the biggest need for them members, or sorting by projects that the org most wants to highlight.

This issue should be focused on prioritizing the display of cards by status in the order delineated in the original opening comment.

@KianBadie : how close are we to a solution eta wise? Do you have enough guidance from @thekaveman ?

joelparkerhenderson commented 4 years ago

This pull request code does the sort by status, and within that by random, during build time: https://github.com/hackforla/website/pull/315

The relevant code:

{% assign status_list = "Active, Rebooting, Completed, On Hold" | split: ", " %}
{% for status in status_list %}
  {% assign projects = site.projects | sample: site.projects.size - 1 | where: "status", status %} 
  {%- for item in projects -%}
    ... 
harishlingam commented 4 years ago

@ExperimentsInHonesty @joelparkerhenderson Please arrange a meeting to discuss this implementation and what if any changes need to be made.

joelparkerhenderson commented 4 years ago

Please arrange a meeting to discuss this implementation and what if any changes need to be made.

@harishlingam Can you say more about why you need a meeting? There's a PR in my comment above that shows what changes I'm doing: https://github.com/hackforla/website/pull/315

I'll add you as PR reviewer now.

If you still feel you want a meeting, can you let me know what days and times are good for you? Or if you want to schedule one with me, my calendar is https://calendly.com/joelparkerhenderson

Thanks!