Skip to content

Commit

Permalink
🔨 remove availableEntities from SelectionArray
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Feb 26, 2025
1 parent bc428b6 commit f7c60d4
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 82 deletions.
4 changes: 3 additions & 1 deletion adminSiteClient/EditorBasicTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ class DimensionSlotView<
@action.bound private updateDefaultSelection() {
const { grapher } = this.props.editor
const { selection } = grapher
const { availableEntityNames, availableEntityNameSet } = selection

const availableEntityNames = grapher.availableEntityNames
const availableEntityNameSet = new Set(grapher.availableEntityNames)

if (grapher.isScatter || grapher.isMarimekko) {
// chart types that display all entities by default shouldn't select any by default
Expand Down
9 changes: 7 additions & 2 deletions adminSiteClient/EditorDataTab.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from "react"
import {
difference,
differenceOfSets,
moveArrayItemToIndex,
omit,
Expand Down Expand Up @@ -179,7 +180,12 @@ export class EntitySelectionSection extends React.Component<{
const { editor } = this
const { grapher } = editor
const { selection } = grapher
const { unselectedEntityNames, selectedEntityNames } = selection
const { selectedEntityNames } = selection

const unselectedEntityNames = difference(
grapher.availableEntityNames,
selectedEntityNames
)

const isEntitySelectionInherited = editor.isPropertyInherited(
"selectedEntityNames"
Expand Down Expand Up @@ -450,7 +456,6 @@ class EntityFilterSection<
@action.bound validateSelectionAndFocus() {
this.editor.removeInvalidSelectedEntityNames()
this.editor.removeInvalidFocusedSeriesNames()
this.editor.grapher.updateAvailableEntitiesOfSelection()
}

@action.bound onExcludeEntity(entityName: string) {
Expand Down
6 changes: 5 additions & 1 deletion packages/@ourworldindata/explorer/src/Explorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
GrapherInterface,
GrapherQueryParams,
GRAPHER_TAB_QUERY_PARAMS,
EntityName,
} from "@ourworldindata/types"
import {
OwidTable,
Expand Down Expand Up @@ -476,10 +477,13 @@ export class Explorer
@action.bound private setGrapherTable(table: OwidTable) {
if (this.grapher) {
this.grapher.inputTable = table
this.grapher.updateAvailableEntitiesOfSelection()
}
}

@computed get availableEntityNames(): EntityName[] {
return this.grapher?.availableEntityNames ?? []
}

private futureGrapherTable = new PromiseSwitcher<OwidTable>({
onResolve: (table) => this.setGrapherTable(table),
onReject: (error) => this.grapher?.setError(error),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { OwidDistinctColorScheme } from "../color/CustomSchemes"

it("can create a new bar chart", () => {
const table = SynthesizeGDPTable({ timeRange: [2000, 2001] })
const selection = new SelectionArray([], table.availableEntities)
const selection = new SelectionArray()
const manager: DiscreteBarChartManager = {
table,
selection,
Expand All @@ -26,7 +26,7 @@ it("can create a new bar chart", () => {
const chart = new DiscreteBarChart({ manager })

expect(chart.failMessage).toBeTruthy()
selection.selectAll()
selection.setSelectedEntities(table.availableEntityNames)
expect(chart.failMessage).toEqual("")

const series = chart.series
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ export class EntityPicker extends React.Component<{
}

@computed private get availableEntitiesForCurrentView(): Set<string> {
if (!this.grapherTable) return this.selection.availableEntityNameSet
if (!this.grapherTable)
return new Set(this.manager.availableEntityNames)
if (!this.manager.requiredColumnSlugs?.length)
return this.grapherTable.availableEntityNameSet
return this.grapherTable.entitiesWith(this.manager.requiredColumnSlugs)
Expand Down Expand Up @@ -209,9 +210,10 @@ export class EntityPicker extends React.Component<{

@computed
private get entitiesWithMetricValue(): EntityOptionWithMetricValue[] {
const { metricTable, selection, localEntityNames } = this
const { metricTable, localEntityNames } = this
const col = this.activePickerMetricColumn
const entityNames = selection.availableEntityNames.slice().sort()
const entityNames =
this.manager.availableEntityNames?.slice().sort() ?? []
return entityNames.map((entityName) => {
const plotValue =
col && metricTable?.has(col.slug)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ColumnSlug } from "@ourworldindata/utils"
import { GrapherAnalytics } from "../../core/GrapherAnalytics"
import { OwidTable } from "@ourworldindata/core-table"
import { CoreColumnDef, SortOrder } from "@ourworldindata/types"
import { CoreColumnDef, EntityName, SortOrder } from "@ourworldindata/types"
import { SelectionArray } from "../../selection/SelectionArray"
import { FocusArray } from "../../focus/FocusArray"

Expand All @@ -21,4 +21,5 @@ export interface EntityPickerManager {
entityType?: string
analytics?: GrapherAnalytics
focusArray?: FocusArray
availableEntityNames?: EntityName[]
}
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ it("can generate a url with country selection even if there is no entity code",
}
const grapher = new Grapher(config)
expect(grapher.queryStr).toBe("")
grapher.selection.selectAll()
grapher.selection.setSelectedEntities(grapher.availableEntityNames)
expect(grapher.queryStr).toContain("AFG")

const config2 = {
Expand All @@ -196,7 +196,7 @@ it("can generate a url with country selection even if there is no entity code",
)!.code = undefined as any
const grapher2 = new Grapher(config2)
expect(grapher2.queryStr).toBe("")
grapher2.selection.selectAll()
grapher2.selection.setSelectedEntities(grapher.availableEntityNames)
expect(grapher2.queryStr).toContain("AFG")
})

Expand Down
25 changes: 12 additions & 13 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -521,10 +521,7 @@ export class Grapher

selection =
this.manager?.selection ??
new SelectionArray(
this.props.selectedEntityNames ?? [],
this.props.table?.availableEntities ?? []
)
new SelectionArray(this.props.selectedEntityNames ?? [])

focusArray = this.manager?.focusArray ?? new FocusArray()

Expand Down Expand Up @@ -1218,8 +1215,6 @@ export class Grapher
// transformation (can happen when columns use targetTime)
this.setDimensionsFromConfigs(dimensions)

this.updateAvailableEntitiesOfSelection()

if (this.manager?.selection?.hasSelection) {
// Selection is managed externally, do nothing.
} else if (this.selection.hasSelection) {
Expand Down Expand Up @@ -1251,10 +1246,6 @@ export class Grapher
this.rebuildInputOwidTable(inputTableTransformer)
}

@action.bound updateAvailableEntitiesOfSelection(): void {
this.selection.setAvailableEntityNames(this.availableEntities)
}

@action.bound private applyOriginalSelectionAsAuthored(): void {
if (this.selectedEntityNames?.length)
this.selection.setSelectedEntities(this.selectedEntityNames)
Expand Down Expand Up @@ -2665,6 +2656,12 @@ export class Grapher
return this.tableForSelection.availableEntities
}

@computed get availableEntityNames(): EntityName[] {
return this.tableForSelection.availableEntities.map(
(entity) => entity.entityName
)
}

private get keyboardShortcuts(): Command[] {
const temporaryFacetTestCommands = range(0, 10).map((num) => {
return {
Expand Down Expand Up @@ -2693,7 +2690,9 @@ export class Grapher
this.selection.clearSelection()
this.focusArray.clear()
} else {
this.selection.selectAll()
this.selection.setSelectedEntities(
this.availableEntityNames
)
}
},
title: this.selection.hasSelection
Expand Down Expand Up @@ -2921,7 +2920,7 @@ export class Grapher
const currentSelection = this.selection.selectedEntityNames.length
const newNum = num ? num : currentSelection ? currentSelection * 2 : 10
this.selection.setSelectedEntities(
sampleFrom(this.selection.availableEntityNames, newNum, Date.now())
sampleFrom(this.availableEntityNames, newNum, Date.now())
)
}

Expand Down Expand Up @@ -3811,7 +3810,7 @@ export class Grapher
// we may still want to show those entities as available in a picker. We also do not want to do things like
// hide the Add Entity button as the user drags the timeline.
@computed private get numSelectableEntityNames(): number {
return this.selection.numAvailableEntityNames
return this.availableEntities.length
}

@computed get entitiesAreCountryLike(): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
import { SelectionArray } from "./SelectionArray"

it("can create a selection", () => {
const selection = new SelectionArray(
[],
[{ entityName: "USA" }, { entityName: "Canada" }]
)
const selection = new SelectionArray()
expect(selection.hasSelection).toEqual(false)

selection.selectAll()
selection.setSelectedEntities(["USA", "Canada"])
expect(selection.hasSelection).toEqual(true)
expect(selection.selectedEntityNames).toEqual(["USA", "Canada"])
})
45 changes: 1 addition & 44 deletions packages/@ourworldindata/grapher/src/selection/SelectionArray.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { EntityCode, EntityId, EntityName } from "@ourworldindata/types"
import { difference } from "@ourworldindata/utils"
import { action, computed, observable } from "mobx"

export interface Entity {
Expand All @@ -9,33 +8,16 @@ export interface Entity {
}

export class SelectionArray {
constructor(
selectedEntityNames: EntityName[] = [],
availableEntities: Entity[] = []
) {
constructor(selectedEntityNames: EntityName[] = []) {
this.selectedEntityNames = selectedEntityNames.slice()
this.availableEntities = availableEntities.slice()
}

@observable selectedEntityNames: EntityName[]
@observable private availableEntities: Entity[]

@computed get availableEntityNames(): string[] {
return this.availableEntities.map((entity) => entity.entityName)
}

@computed get availableEntityNameSet(): Set<string> {
return new Set(this.availableEntityNames)
}

@computed get hasSelection(): boolean {
return this.selectedEntityNames.length > 0
}

@computed get unselectedEntityNames(): string[] {
return difference(this.availableEntityNames, this.selectedEntityNames)
}

@computed get numSelectedEntities(): number {
return this.selectedEntityNames.length
}
Expand All @@ -55,20 +37,6 @@ export class SelectionArray {
return this
}

@action.bound addAvailableEntityNames(entities: Entity[]): this {
this.availableEntities.push(...entities)
return this
}

@action.bound setAvailableEntityNames(entities: Entity[]): this {
this.availableEntities = entities
return this
}

@action.bound selectAll(): this {
return this.addToSelection(this.unselectedEntityNames)
}

@action.bound clearSelection(): void {
this.selectedEntityNames = []
}
Expand All @@ -79,10 +47,6 @@ export class SelectionArray {
: this.selectEntity(entityName)
}

@computed get numAvailableEntityNames(): number {
return this.availableEntityNames.length
}

@action.bound selectEntity(entityName: EntityName): this {
if (!this.selectedSet.has(entityName))
return this.addToSelection([entityName])
Expand All @@ -107,11 +71,4 @@ export class SelectionArray {
)
return this
}

// Mainly for testing
@action.bound selectSample(howMany = 1): this {
return this.setSelectedEntities(
this.availableEntityNames.slice(0, howMany)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,7 @@ describe("series naming in multi-column mode", () => {
})

it("combines entity and column name if multiple entities are selected and multi entity selection is disabled", () => {
const selection = new SelectionArray(
table.availableEntityNames,
table.availableEntities
)
const selection = new SelectionArray(table.availableEntityNames)
const manager = {
table,
canSelectMultipleEntities: false,
Expand Down Expand Up @@ -242,10 +239,7 @@ describe("colors", () => {
]
)

const selection = new SelectionArray(
["usa", "canada"],
[{ entityName: "usa" }, { entityName: "canada" }]
)
const selection = new SelectionArray(["usa", "canada"])
const manager: SlopeChartManager = {
yColumnSlugs: ["gdp", "pop"],
table: table,
Expand Down

0 comments on commit f7c60d4

Please sign in to comment.