Skip to content

Commit

Permalink
Refactor counters (#703)
Browse files Browse the repository at this point in the history 
In preparation for changing the table view text, then the ReformStatus
eventually changing the counter.

Changes:

* Remove the number of records, which will not be in the upcoming table
view revamp. It's too nitty-gritty for users.
* Use helper functions so that we can early return
* Test the helper functions rather than more integration test style. It
makes it easier to test every permutation and makes them less verbose.
  • Loading branch information
@Eric-Arellano
Eric-Arellano authored Jan 3, 2025
1 parent dab8699 commit d29c27a
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 555 deletions.
13 changes: 0 additions & 13 deletions src/js/FilterState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ interface CacheEntry {
matchedCountries: Set<string>;
matchedPolicyTypesForAnyPolicy: Set<PolicyType>;
matchedPlaceTypes: Set<PlaceType>;
numMatchedPolicyRecordsForSinglePolicy: number;
}

export class PlaceFilterManager {
Expand All @@ -111,13 +110,6 @@ export class PlaceFilterManager {
return this.ensureCache().matchedPlaces;
}

/// The number of matching policy records, given that a place may have >1 policy record.
///
/// Note that this will be zero with 'any parking reform' and 'search'.
get numMatchedPolicyRecords(): number {
return this.ensureCache().numMatchedPolicyRecordsForSinglePolicy;
}

get placeIds(): Set<PlaceId> {
return new Set(Object.keys(this.matchedPlaces));
}
Expand Down Expand Up @@ -168,16 +160,12 @@ export class PlaceFilterManager {
const matchedCountries = new Set<string>();
const matchedPolicyTypes = new Set<PolicyType>();
const matchedPlaceTypes = new Set<PlaceType>();
let numMatchedPolicyRecords = 0;
for (const placeId in this.entries) {
const match = this.getPlaceMatch(placeId);
if (!match) continue;
matchedPlaces[placeId] = match;
matchedCountries.add(this.entries[placeId].place.country);
matchedPlaceTypes.add(this.entries[placeId].place.type);
if (match.type === "single policy") {
numMatchedPolicyRecords += match.matchingIndexes.length;
}
if (match.type === "any") {
if (match.hasAddMax) matchedPolicyTypes.add("add parking maximums");
if (match.hasReduceMin)
Expand All @@ -192,7 +180,6 @@ export class PlaceFilterManager {
matchedCountries,
matchedPolicyTypesForAnyPolicy: matchedPolicyTypes,
matchedPlaceTypes,
numMatchedPolicyRecordsForSinglePolicy: numMatchedPolicyRecords,
};
return this.cache;
}
Expand Down
198 changes: 103 additions & 95 deletions src/js/counters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,11 @@ import { FilterState, PlaceFilterManager } from "./FilterState";
import { PlaceType, PolicyType } from "./types";
import { joinWithConjunction } from "./data";

export function determineHtml(
view: "table" | "map",
state: FilterState,
export function determinePlaceDescription(
numPlaces: number,
numPolicyRecords: number,
matchedPolicyTypes: Set<PolicyType>,
matchedCountries: Set<string>,
matchedPlaceTypes: Set<PlaceType>,
): string {
if (!numPlaces) {
return "No places selected — use the filter or search icons";
}
if (state.searchInput) {
return `Showing ${state.searchInput} from search — <a class="counter-search-reset" role="button" aria-label="reset search">reset</a>`;
}

let country =
matchedCountries.size === 1
? Array.from(matchedCountries)[0]
Expand All @@ -28,105 +17,128 @@ export function determineHtml(
country = `the ${country}`;
}

let placeDescription;
if (isEqual(matchedPlaceTypes, new Set(["city"]))) {
const label = numPlaces === 1 ? "city" : "cities";
placeDescription = `${label} in ${country}`;
return `${numPlaces} ${label} in ${country}`;
} else if (isEqual(matchedPlaceTypes, new Set(["county"]))) {
const label = numPlaces === 1 ? "county" : "counties";
placeDescription = `${label} in ${country}`;
return `${numPlaces} ${label} in ${country}`;
} else if (isEqual(matchedPlaceTypes, new Set(["state"]))) {
const label = numPlaces === 1 ? "state" : "states";
placeDescription = `${label} in ${country}`;
return `${numPlaces} ${label} in ${country}`;
} else if (isEqual(matchedPlaceTypes, new Set(["country"]))) {
placeDescription = numPlaces === 1 ? "country" : "countries";
return numPlaces === 1 ? "1 country" : `${numPlaces} countries`;
} else {
const label = numPlaces === 1 ? "place" : "places";
placeDescription = `${label} in ${country}`;
}
const prefix = `Showing ${numPlaces} ${placeDescription} with`;

// We only show the number of policy records when it's useful information to the user
// because it would otherwise be noisy.
const recordsWord = numPolicyRecords === 1 ? "record" : "records";
const showRecords = view === "table" && numPlaces !== numPolicyRecords;
const multipleRecordsExplanation =
"because some places have multiple records";

if (state.policyTypeFilter === "legacy reform") {
const suffix = state.allMinimumsRemovedToggle
? "all parking minimums removed"
: "parking reforms";
return `${prefix} ${suffix}`;
return `${numPlaces} ${label} in ${country}`;
}
}

if (state.policyTypeFilter === "any parking reform") {
// We customize the text based on which policy changes are selected.
let suffix;
if (state.allMinimumsRemovedToggle) {
suffix = isEqual(
state.includedPolicyChanges,
new Set(["add parking maximums"]),
)
? "both all parking minimums removed and parking maximums added"
: "all parking minimums removed";
} else {
const policyDescriptionMap: Record<PolicyType, string> = {
"add parking maximums": "parking maximums added",
"reduce parking minimums": "parking minimums reduced",
"remove parking minimums": "parking minimums removed",
};
const policyDescriptions = Array.from(state.includedPolicyChanges)
.filter((policy) => matchedPolicyTypes.has(policy as PolicyType))
.map((policy) => policyDescriptionMap[policy as PolicyType])
.sort()
.reverse();
if (!policyDescriptions.length) {
throw new Error(
`Expected state.includedPolicyChanges to be set: ${JSON.stringify(state)}`,
);
}
suffix = joinWithConjunction(policyDescriptions, "or");
}
export function determineLegacy(
placeDescription: string,
allMinimumsRemovedToggle: boolean,
): string {
const suffix = allMinimumsRemovedToggle
? `all parking minimums removed`
: `parking reforms`;
return `Showing ${placeDescription} with ${suffix}`;
}

export function determineAnyReform(
placeDescription: string,
matchedPolicyTypes: Set<PolicyType>,
allMinimumsRemovedToggle: boolean,
statePolicyTypes: Set<string>,
): string {
const prefix = `Showing ${placeDescription} with`;

if (allMinimumsRemovedToggle) {
const suffix = isEqual(statePolicyTypes, new Set(["add parking maximums"]))
? "both all parking minimums removed and parking maximums added"
: "all parking minimums removed";
return `${prefix} ${suffix}`;
}

if (state.policyTypeFilter === "reduce parking minimums") {
const firstSentence = `${prefix} parking minimums reduced`;
return showRecords
? `${firstSentence}. Matched ${numPolicyRecords} total policy ${recordsWord} ${multipleRecordsExplanation}.`
: firstSentence;
const policyDescriptionMap: Record<PolicyType, string> = {
"add parking maximums": "parking maximums added",
"reduce parking minimums": "parking minimums reduced",
"remove parking minimums": "parking minimums removed",
};
const policyDescriptions = Array.from(statePolicyTypes)
.filter((policy) => matchedPolicyTypes.has(policy as PolicyType))
.map((policy) => policyDescriptionMap[policy as PolicyType])
.sort()
.reverse();
if (!policyDescriptions.length) {
throw new Error(`Expected state.includedPolicyChanges to be set`);
}
const suffix = joinWithConjunction(policyDescriptions, "or");
return `${prefix} ${suffix}`;
}

if (state.policyTypeFilter === "add parking maximums") {
let firstSentenceSuffix;
let secondSentence;
if (state.allMinimumsRemovedToggle) {
firstSentenceSuffix =
"both all parking minimums removed and parking maximums added";
secondSentence = `Matched ${numPolicyRecords} total parking maximum policy ${recordsWord} ${multipleRecordsExplanation}`;
} else {
firstSentenceSuffix = "parking maximums added";
secondSentence = `Matched ${numPolicyRecords} total policy ${recordsWord} ${multipleRecordsExplanation}`;
}
const firstSentence = `${prefix} ${firstSentenceSuffix}`;
return showRecords ? `${firstSentence}. ${secondSentence}.` : firstSentence;
}
export function determineReduceMin(placeDescription: string): string {
return `Showing ${placeDescription} with parking minimums reduced`;
}

if (state.policyTypeFilter === "remove parking minimums") {
// It's not necessary to say the # of policy records when allMinimumsRemovedToggle is true because the place should have
// only one removal policy record that is citywide & all land uses.
if (state.allMinimumsRemovedToggle) {
return `${prefix} all parking minimums removed`;
}
export function determineAddMax(
placeDescription: string,
allMinimumsRemovedToggle: boolean,
): string {
const suffix = allMinimumsRemovedToggle
? "both all parking minimums removed and parking maximums added"
: "parking maximums added";
return `Showing ${placeDescription} with ${suffix}`;
}

const firstSentence = `${prefix} parking minimums removed`;
return showRecords
? `${firstSentence}. Matched ${numPolicyRecords} total policy ${recordsWord} ${multipleRecordsExplanation}.`
: firstSentence;
export function determineRmMin(
placeDescription: string,
allMinimumsRemovedToggle: boolean,
): string {
const suffix = allMinimumsRemovedToggle
? `all parking minimums removed`
: `parking minimums removed`;
return `Showing ${placeDescription} with ${suffix}`;
}

export function determineHtml(
state: FilterState,
numPlaces: number,
matchedPolicyTypes: Set<PolicyType>,
matchedCountries: Set<string>,
matchedPlaceTypes: Set<PlaceType>,
): string {
if (!numPlaces) {
return "No places selected — use the filter or search icons";
}
if (state.searchInput) {
return `Showing ${state.searchInput} from search — <a class="counter-search-reset" role="button" aria-label="reset search">reset</a>`;
}

throw new Error("unreachable");
const placeDescription = determinePlaceDescription(
numPlaces,
matchedCountries,
matchedPlaceTypes,
);

switch (state.policyTypeFilter) {
case "legacy reform":
return determineLegacy(placeDescription, state.allMinimumsRemovedToggle);
case "any parking reform":
return determineAnyReform(
placeDescription,
matchedPolicyTypes,
state.allMinimumsRemovedToggle,
state.includedPolicyChanges,
);
case "reduce parking minimums":
return determineReduceMin(placeDescription);
case "add parking maximums":
return determineAddMax(placeDescription, state.allMinimumsRemovedToggle);
case "remove parking minimums":
return determineRmMin(placeDescription, state.allMinimumsRemovedToggle);
default:
throw new Error("unreachable");
}
}

function setUpResetButton(
Expand All @@ -153,19 +165,15 @@ export default function initCounters(manager: PlaceFilterManager): void {

manager.subscribe("update counters", (state) => {
mapCounter.innerHTML = determineHtml(
"map",
state,
manager.placeIds.size,
manager.numMatchedPolicyRecords,
manager.matchedPolicyTypes,
manager.matchedCountries,
manager.matchedPlaceTypes,
);
tableCounter.innerHTML = determineHtml(
"table",
state,
manager.placeIds.size,
manager.numMatchedPolicyRecords,
manager.matchedPolicyTypes,
manager.matchedCountries,
manager.matchedPlaceTypes,
Expand Down
Loading

0 comments on commit d29c27a

Please sign in to comment.