Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fix 1444 clean-up some old code" #3941

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions adminSiteClient/EditorScatterTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export class EditorScatterTab extends React.Component<{ grapher: Grapher }> {
this.props.grapher.hideTimeline = value || undefined
}

@action.bound onToggleHideLinesOutsideTolerance(value: boolean) {
this.props.grapher.hideLinesOutsideTolerance = value || undefined
}

@action.bound onToggleHideScatterLabels(value: boolean) {
this.props.grapher.hideScatterLabels = value || undefined
}
Expand Down Expand Up @@ -142,6 +146,11 @@ export class EditorScatterTab extends React.Component<{ grapher: Grapher }> {
value={!!grapher.hideTimeline}
onValue={this.onToggleHideTimeline}
/>
<Toggle
label="Hide entities without data for full time span (within tolerance)"
value={!!grapher.hideLinesOutsideTolerance}
onValue={this.onToggleHideLinesOutsideTolerance}
/>
<Toggle
label="Hide connected scatter lines"
value={!!grapher.hideConnectedScatterLines}
Expand Down
2 changes: 1 addition & 1 deletion adminSiteClient/GrapherConfigGridEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd

async getFieldDefinitions() {
const json = await fetch(
"https://files.ourworldindata.org/schemas/grapher-schema.005.json"
"https://files.ourworldindata.org/schemas/grapher-schema.004.json"
).then((response) => response.json())
const fieldDescriptions = extractFieldDescriptionsFromSchema(json)
runInAction(() => {
Expand Down
13 changes: 1 addition & 12 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ const saveGrapher = async (
referencedVariablesMightChange = true,
}: {
user: DbPlainUser
newConfig: GrapherInterface // Note that it is valid for newConfig to be of an older schema version which means that GrapherInterface as a type is slightly misleading
newConfig: GrapherInterface
existingConfig?: GrapherInterface
// if undefined, keep inheritance as is.
// if true or false, enable or disable inheritance
Expand Down Expand Up @@ -582,17 +582,6 @@ const saveGrapher = async (
// if the schema version is missing, assume it's the latest
if (newConfig.$schema === undefined) {
newConfig.$schema = defaultGrapherConfig.$schema
} else if (
newConfig.$schema ===
"https://files.ourworldindata.org/schemas/grapher-schema.004.json"
) {
// TODO: find a more principled way to do schema upgrades

// grapher-schema.004 -> grapher-schema.005 removed the obsolete hideLinesOutsideTolerance field
const configForMigration = newConfig as any
delete configForMigration.hideLinesOutsideTolerance
configForMigration.$schema = defaultGrapherConfig.$schema
newConfig = configForMigration
}

// add the isPublished field if is missing
Expand Down

This file was deleted.

6 changes: 6 additions & 0 deletions devTools/schemaProcessor/columns.json
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,12 @@
"default": false,
"editor": "checkbox"
},
{
"type": "boolean",
"pointer": "/hideLinesOutsideTolerance",
"default": false,
"editor": "checkbox"
},
{
"type": "boolean",
"pointer": "/hideTotalValueLabel",
Expand Down
1 change: 1 addition & 0 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ export class Grapher
@observable.ref isPublished?: boolean = undefined
@observable.ref baseColorScheme?: ColorSchemeName = undefined
@observable.ref invertColorScheme?: boolean = undefined
@observable.ref hideLinesOutsideTolerance?: boolean = undefined
@observable hideConnectedScatterLines?: boolean = undefined // Hides lines between points when timeline spans multiple years. Requested by core-econ for certain charts
@observable
scatterPointLabelStrategy?: ScatterPointLabelStrategy = undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ describe("series transformations", () => {
const chart = new ScatterPlotChart({
manager: {
...manager,
hideLinesOutsideTolerance: true,
startTime: 2000,
endTime: 2003,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ export class ScatterPlotChart
@computed get transformedTable(): OwidTable {
let table = this.transformedTableFromGrapher
if (
this.manager.hideLinesOutsideTolerance &&
this.manager.startTime !== undefined &&
this.manager.endTime !== undefined
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface ScatterPlotManager extends ChartManager {
includedEntities?: EntityId[]
excludedEntities?: EntityId[]
backgroundSeriesLimit?: number
hideLinesOutsideTolerance?: boolean
startTime?: Time
endTime?: Time
hasTimeline?: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { GrapherInterface } from "@ourworldindata/types"

export const defaultGrapherConfig = {
$schema: "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
$schema: "https://files.ourworldindata.org/schemas/grapher-schema.004.json",
map: {
projection: "World",
hideTimeline: false,
Expand Down Expand Up @@ -68,6 +68,7 @@ export const defaultGrapherConfig = {
showNoDataArea: true,
zoomToSelection: false,
showYearLabels: false,
hideLinesOutsideTolerance: false,
hideTotalValueLabel: false,
hideScatterLabels: false,
sortBy: "total",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
$schema: "http://json-schema.org/draft-07/schema#"
$id: "https://files.ourworldindata.org/schemas/grapher-schema.004.json"
# if you update the required keys, make sure that the mergeGrapherConfigs and
# diffGrapherConfigs functions both reflect this change
$id: "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
required:
- $schema
- dimensions
Expand All @@ -14,13 +14,13 @@ properties:
type: string
description: Url of the concrete schema version to use to validate this document
format: uri
default: "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
default: "https://files.ourworldindata.org/schemas/grapher-schema.004.json"
# for now, we only validate configs in our database using this schema.
# since we expect all configs in our database to be valid against the latest schema,
# we restrict the $schema field to a single value, the latest schema version.
# if we ever need to validate configs against multiple schema versions,
# we can remove this constraint.
const: "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
const: "https://files.ourworldindata.org/schemas/grapher-schema.004.json"
id:
type: integer
description: Internal DB id. Useful internally for OWID but not required if just using grapher directly.
Expand Down Expand Up @@ -443,6 +443,10 @@ properties:
type: boolean
default: false
description: Whether to show year labels in bar charts
hideLinesOutsideTolerance:
type: boolean
default: false
description: Hide entities without data for full time span (within tolerance)
hideTotalValueLabel:
type: boolean
default: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ export interface GrapherInterface extends SortConfig {
isPublished?: boolean
baseColorScheme?: ColorSchemeName
invertColorScheme?: boolean
hideLinesOutsideTolerance?: boolean
hideConnectedScatterLines?: boolean // Hides lines between points when timeline spans multiple years. Requested by core-econ for certain charts
hideScatterLabels?: boolean
scatterPointLabelStrategy?: ScatterPointLabelStrategy
Expand Down Expand Up @@ -655,6 +656,7 @@ export const grapherKeysToSerialize = [
"isPublished",
"baseColorScheme",
"invertColorScheme",
"hideLinesOutsideTolerance",
"hideConnectedScatterLines",
"hideScatterLabels",
"scatterPointLabelStrategy",
Expand Down