Improve scorecard design & mention status (#701)

We now always use a bullet list for the reform types. The details link comes at the end: ![image](https://github.com/user-attachments/assets/87a73fcf-5006-47c5-9541-51b62cd9a1de) ![image](https://github.com/user-attachments/assets/a71356f0-93ea-4179-ac1d-7655a3b204ef) If at least one policy record is proposed or repealed (vs adopted), we show the status with everything: ![image](https://github.com/user-attachments/assets/20e02f90-701c-4e79-801a-11a418f1dd82) ![image](https://github.com/user-attachments/assets/accee630-a0d2-4d96-a7f1-79c30c51cd27)
ParkingReformNetwork · Jan 3, 2025 · dab8699 · dab8699
1 parent 12910a3
commit dab8699
Show file tree

Hide file tree

Showing 7 changed files with 169 additions and 34 deletions.
diff --git a/src/js/FilterState.ts b/src/js/FilterState.ts
@@ -9,7 +9,7 @@ import {
   UNKNOWN_YEAR,
 } from "./types";
 import Observable from "./Observable";
-import { determinePolicyTypes, getFilteredIndexes } from "./data";
+import { determineAdoptedPolicyTypes, getFilteredIndexes } from "./data";
 
 export const POPULATION_INTERVALS: Array<[string, number]> = [
   ["100", 100],
@@ -269,7 +269,7 @@ export class PlaceFilterManager {
     }
 
     if (filterState.policyTypeFilter === "any parking reform") {
-      const policyTypes = determinePolicyTypes(entry, { onlyAdopted: true });
+      const policyTypes = determineAdoptedPolicyTypes(entry);
       const isPolicyType = policyTypes.some((v) =>
         filterState.includedPolicyChanges.has(v),
       );

diff --git a/src/js/counters.ts b/src/js/counters.ts
@@ -2,6 +2,7 @@ import { isEqual } from "lodash-es";
 
 import { FilterState, PlaceFilterManager } from "./FilterState";
 import { PlaceType, PolicyType } from "./types";
+import { joinWithConjunction } from "./data";
 
 export function determineHtml(
   view: "table" | "map",
@@ -85,12 +86,7 @@ export function determineHtml(
           `Expected state.includedPolicyChanges to be set: ${JSON.stringify(state)}`,
         );
       }
-      if (policyDescriptions.length <= 2) {
-        suffix = policyDescriptions.join(" or ");
-      } else {
-        const lastTerm = policyDescriptions.pop();
-        suffix = `${policyDescriptions.join(", ")}, or ${lastTerm}`;
-      }
+      suffix = joinWithConjunction(policyDescriptions, "or");
     }
     return `${prefix} ${suffix}`;
   }

diff --git a/src/js/data.ts b/src/js/data.ts
@@ -9,6 +9,7 @@ import {
   ProcessedPlace,
   ProcessedCorePolicy,
   ProcessedLegacyReform,
+  ReformStatus,
 } from "./types";
 
 export const COUNTRY_MAPPING: Partial<Record<string, string>> = {
@@ -59,16 +60,12 @@ export function numberOfPolicyRecords(
   );
 }
 
-export function determinePolicyTypes(
+export function determineAdoptedPolicyTypes(
   entry: RawCoreEntry | ProcessedCoreEntry,
-  options: { onlyAdopted: boolean },
 ): PolicyType[] {
   const hasPolicy = (
     policies: ProcessedCorePolicy[] | RawCorePolicy[] | undefined,
-  ) =>
-    !!policies?.filter((policy) =>
-      options.onlyAdopted ? policy.status === "adopted" : true,
-    ).length;
+  ) => !!policies?.filter((policy) => policy.status === "adopted").length;
 
   const result: PolicyType[] = [];
   if (hasPolicy(entry.add_max)) result.push("add parking maximums");
@@ -77,6 +74,19 @@ export function determinePolicyTypes(
   return result;
 }
 
+export function determinePolicyTypeStatuses(
+  entry: RawCoreEntry | ProcessedCoreEntry,
+): Record<PolicyType, Set<ReformStatus>> {
+  const getStatuses = (
+    policies: ProcessedCorePolicy[] | RawCorePolicy[] | undefined,
+  ) => new Set(policies?.map((policy) => policy.status) ?? []);
+  return {
+    "add parking maximums": getStatuses(entry.add_max),
+    "reduce parking minimums": getStatuses(entry.reduce_min),
+    "remove parking minimums": getStatuses(entry.rm_min),
+  };
+}
+
 function processPolicy(raw: RawCorePolicy): ProcessedCorePolicy {
   return {
     ...raw,
@@ -166,3 +176,15 @@ export function getFilteredIndexes<T>(
     return indexes;
   }, []);
 }
+
+export function joinWithConjunction(
+  items: string[],
+  conjunction: "and" | "or",
+): string {
+  if (items.length <= 2) {
+    return items.join(` ${conjunction} `);
+  }
+  const lastItem = items[items.length - 1];
+  const priorItems = items.slice(0, -1);
+  return `${priorItems.join(", ")}, ${conjunction} ${lastItem}`;
+}
diff --git a/src/js/scorecard.ts b/src/js/scorecard.ts
@@ -5,7 +5,7 @@ import type { ProcessedCoreEntry, PlaceId } from "./types";
 import Observable from "./Observable";
 import { PlaceFilterManager } from "./FilterState";
 import { ViewStateObservable } from "./viewToggle";
-import { determinePolicyTypes } from "./data";
+import { determinePolicyTypeStatuses, joinWithConjunction } from "./data";
 
 function generateScorecardLegacy(
   entry: ProcessedCoreEntry,
@@ -36,21 +36,34 @@ function generateScorecardLegacy(
     `;
 }
 
-function generateScorecardRevamp(
+export function generateScorecardRevamp(
   entry: ProcessedCoreEntry,
   placeId: PlaceId,
 ): string {
-  const policyTypes = determinePolicyTypes(entry, { onlyAdopted: false });
-  let singlePolicyType = "";
-  let multiplePolicyTypes = "";
-  if (policyTypes.length === 1) {
-    singlePolicyType = `<li>Reform type: ${policyTypes[0]}</li>`;
-  } else {
-    const policies = policyTypes
-      .map((type) => `<li>${capitalize(type)}</li>`)
-      .join("");
-    multiplePolicyTypes = `<div>Reform types:</div><ul>${policies}</ul>`;
-  }
+  const policyToStatuses = determinePolicyTypeStatuses(entry);
+  // If at least one policy record is proposed or repealed, we mention
+  // the ReformStatus with every policy type so that people don't incorrectly
+  // think a record was adopted when it wasn't.
+  const needsStatusLabels = Object.values(policyToStatuses).some(
+    (statuses) => statuses.has("proposed") || statuses.has("repealed"),
+  );
+
+  const policies = Object.entries(policyToStatuses)
+    .filter(([, statuses]) => statuses.size)
+    .map(([policyType, statusesSet]) => {
+      let suffix = "";
+      if (needsStatusLabels) {
+        const statuses = joinWithConjunction(
+          Array.from(statusesSet).sort(),
+          "and",
+        );
+        suffix = ` (${statuses})`;
+      }
+      const val = capitalize(`${policyType}${suffix}`);
+      return `<li>${val}</li>`;
+    })
+    .join("");
+  const policyTypesHtml = `<div>Reform types:</div><ul>${policies}</ul>`;
 
   const allMinimumsRemoved = entry.place.repeal
     ? "<li>All parking minimums removed</li>"
@@ -68,14 +81,13 @@ function generateScorecardRevamp(
       </button>
     </header>
     <ul>
-      <li><a class="external-link" target="_blank" href=${
-        entry.place.url
-      }>Reform details and citations <i aria-hidden="true" class="fa-solid fa-arrow-right"></i></a></li>
       <li>${entry.place.pop.toLocaleString()} residents</li>
       ${allMinimumsRemoved}
-      ${singlePolicyType}
     </ul>
-    ${multiplePolicyTypes}
+    ${policyTypesHtml}
+    <a class="external-link" target="_blank" href=${
+      entry.place.url
+    }>Details and citations <i aria-hidden="true" class="fa-solid fa-arrow-right"></i></a>
     `;
 }
 

diff --git a/src/js/table.ts b/src/js/table.ts
@@ -14,7 +14,7 @@ import {
 import { PlaceFilterManager, PolicyTypeFilter } from "./FilterState";
 import { Date, ProcessedCorePolicy } from "./types";
 import { ViewStateObservable } from "./viewToggle";
-import { determinePolicyTypes } from "./data";
+import { determineAdoptedPolicyTypes } from "./data";
 
 function formatBoolean(cell: CellComponent): string {
   const v = cell.getValue() as boolean;
@@ -180,7 +180,7 @@ export default function initTable(
 
     if (!options.revampEnabled) return;
 
-    const policyTypes = determinePolicyTypes(entry, { onlyAdopted: true });
+    const policyTypes = determineAdoptedPolicyTypes(entry);
     dataAnyReform.push({
       ...common,
       reduceMin: policyTypes.includes("reduce parking minimums"),

diff --git a/tests/app/data.test.ts b/tests/app/data.test.ts
@@ -4,6 +4,7 @@ import {
   escapePlaceId,
   placeIdToUrl,
   getFilteredIndexes,
+  joinWithConjunction,
 } from "../../src/js/data";
 
 test("escapePlaceID", () => {
@@ -22,3 +23,25 @@ test("getFilteredIndexes", () => {
   expect(getFilteredIndexes(["a", "b", "c"], (x) => x === "b")).toEqual([1]);
   expect(getFilteredIndexes(["a", "b", "c"], (x) => x === "z")).toEqual([]);
 });
+
+test("joinWithConjunction", () => {
+  expect(joinWithConjunction([], "and")).toEqual("");
+  expect(joinWithConjunction(["apple"], "and")).toEqual("apple");
+
+  expect(joinWithConjunction(["apple", "banana"], "and")).toEqual(
+    "apple and banana",
+  );
+  expect(joinWithConjunction(["read", "write"], "or")).toEqual("read or write");
+
+  expect(joinWithConjunction(["apple", "banana", "orange"], "and")).toEqual(
+    "apple, banana, and orange",
+  );
+  expect(
+    joinWithConjunction(["read", "write", "execute", "delete"], "or"),
+  ).toEqual("read, write, execute, or delete");
+
+  expect(
+    joinWithConjunction(["  space  ", "tab\t", "\nnewline"], "and"),
+  ).toEqual("  space  , tab\t, and \nnewline");
+  expect(joinWithConjunction(["!", "@", "#"], "or")).toEqual("!, @, or #");
+});
diff --git a/tests/app/scorecard.test.ts b/tests/app/scorecard.test.ts
@@ -1,5 +1,12 @@
 import { expect, test } from "@playwright/test";
+
 import { loadMap } from "./utils";
+import { generateScorecardRevamp } from "../../src/js/scorecard";
+import {
+  ProcessedCoreEntry,
+  ProcessedCorePolicy,
+  ProcessedPlace,
+} from "../../src/js/types";
 
 test("scorecard pops up and closes", async ({ page }) => {
   await loadMap(page);
@@ -35,3 +42,78 @@ test("scorecard pops up and closes", async ({ page }) => {
   await page.click("#map");
   expect(await scorecardIsVisible()).toBe(false);
 });
+
+test("generateScorecardRevamp", () => {
+  const place: ProcessedPlace = {
+    name: "My City",
+    state: "AZ",
+    country: "US",
+    type: "city",
+    pop: 245132,
+    repeal: true,
+    coord: [0, 0],
+    url: "https://my-site.org",
+  };
+  const policy: ProcessedCorePolicy = {
+    status: "adopted",
+    summary: "",
+    scope: [],
+    land: [],
+    date: null,
+  };
+  const entry: ProcessedCoreEntry = {
+    place,
+    unifiedPolicy: { ...policy, policy: [] },
+    add_max: [policy],
+  };
+  expect(generateScorecardRevamp(entry, "My City, AZ")).toEqual(
+    `
+    <header class="scorecard-header">
+      <h2 class="scorecard-title">My City, AZ</h2>
+      <button
+        class="scorecard-close-icon-container"
+        title="close the place details popup"
+        aria-label="close the place details popup"
+      >
+        <i class="fa-regular fa-circle-xmark" aria-hidden="true"></i>
+      </button>
+    </header>
+    <ul>
+      <li>245,132 residents</li>
+      <li>All parking minimums removed</li>
+    </ul>
+    <div>Reform types:</div><ul><li>Add parking maximums</li></ul>
+    <a class="external-link" target="_blank" href=https://my-site.org>Details and citations <i aria-hidden="true" class="fa-solid fa-arrow-right"></i></a>
+    `,
+  );
+
+  const repealed: ProcessedCoreEntry = {
+    place: { ...place, repeal: false },
+    unifiedPolicy: { ...policy, policy: [] },
+    add_max: [
+      { ...policy, status: "repealed" },
+      { ...policy, status: "proposed" },
+    ],
+    rm_min: [policy],
+  };
+  expect(generateScorecardRevamp(repealed, "My City, AZ")).toEqual(
+    `
+    <header class="scorecard-header">
+      <h2 class="scorecard-title">My City, AZ</h2>
+      <button
+        class="scorecard-close-icon-container"
+        title="close the place details popup"
+        aria-label="close the place details popup"
+      >
+        <i class="fa-regular fa-circle-xmark" aria-hidden="true"></i>
+      </button>
+    </header>
+    <ul>
+      <li>245,132 residents</li>
+      
+    </ul>
+    <div>Reform types:</div><ul><li>Add parking maximums (proposed and repealed)</li><li>Remove parking minimums (adopted)</li></ul>
+    <a class="external-link" target="_blank" href=https://my-site.org>Details and citations <i aria-hidden="true" class="fa-solid fa-arrow-right"></i></a>
+    `,
+  );
+});