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.

Sign up for GitHub

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

🔨 (grapher) turn related questions into an object / TAS-676 #4108

Closed
wants to merge 2 commits into from
Closed

🔨 (grapher) turn related questions into an object / TAS-676 #4108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c89b72
🔨 remove default layer from chart configs
sophiamersmann Nov 1, 2024
4b49cf0
🔨 (grapher) turn related questions into an object
sophiamersmann Nov 4, 2024
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
16 changes: 13 additions & 3 deletions adminSiteClient/AbstractChartEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,19 @@ export abstract class AbstractChartEditor<
return mergeGrapherConfigs(parentConfig ?? {}, patchConfig)
}

/** live-updating full config */
/** live-updating config */
@computed get liveConfig(): GrapherInterface {
return this.grapher.object
}

@computed get liveConfigWithDefaults(): GrapherInterface {
return mergeGrapherConfigs(defaultGrapherConfig, this.liveConfig)
}

/** patch config merged with parent config */
@computed get fullConfig(): GrapherInterface {
return mergeGrapherConfigs(defaultGrapherConfig, this.grapher.object)
if (!this.activeParentConfig) return this.liveConfig
return mergeGrapherConfigs(this.activeParentConfig, this.liveConfig)
}

/** parent config currently applied to grapher */
Expand All @@ -103,7 +113,7 @@ export abstract class AbstractChartEditor<
/** patch config of the chart that is written to the db on save */
@computed get patchConfig(): GrapherInterface {
return diffGrapherConfigs(
this.fullConfig,
this.liveConfigWithDefaults,
this.activeParentConfigWithDefaults ?? defaultGrapherConfig
)
}
Expand Down
2 changes: 1 addition & 1 deletion adminSiteClient/ChartEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {

/** parent variable id, derived from the config */
@computed get parentVariableId(): number | undefined {
return getParentVariableIdFromChartConfig(this.fullConfig)
return getParentVariableIdFromChartConfig(this.liveConfig)
}

@computed get availableTabs(): EditorTab[] {
Expand Down
105 changes: 44 additions & 61 deletions adminSiteClient/EditorTextTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"
import { faMinus } from "@fortawesome/free-solid-svg-icons"
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js"
import { LogoOption, RelatedQuestionsConfig } from "@ourworldindata/types"
import { LogoOption } from "@ourworldindata/types"
import { getErrorMessageRelatedQuestionUrl } from "@ourworldindata/grapher"
import { slugify } from "@ourworldindata/utils"
import { action, computed } from "mobx"
Expand Down Expand Up @@ -37,21 +37,6 @@ export class EditorTextTab<
}
}

@action.bound onAddRelatedQuestion() {
const { grapher } = this.props.editor
if (!grapher.relatedQuestions) grapher.relatedQuestions = []
grapher.relatedQuestions.push({
text: "",
url: "",
})
}

@action.bound onRemoveRelatedQuestion(idx: number) {
const { grapher } = this.props.editor
if (!grapher.relatedQuestions) grapher.relatedQuestions = []
grapher.relatedQuestions.splice(idx, 1)
}

@action.bound onToggleTitleAnnotationEntity(value: boolean) {
const { grapher } = this.props.editor
grapher.hideAnnotationFieldsInTitle ??= {}
Expand Down Expand Up @@ -86,7 +71,6 @@ export class EditorTextTab<
render() {
const { editor } = this.props
const { grapher, features } = editor
const { relatedQuestions = [] } = grapher

return (
<div className="EditorTextTab">
Expand Down Expand Up @@ -250,50 +234,49 @@ export class EditorTextTab<
</Section>

<Section name="Related">
{relatedQuestions.map(
(question: RelatedQuestionsConfig, idx: number) => (
<div key={idx}>
<TextField
label="Related question"
value={question.text}
onValue={action((value: string) => {
question.text = value
})}
placeholder="e.g. How did countries respond to the pandemic?"
helpText="Short question promoting exploration of related content"
softCharacterLimit={50}
/>
{question.text && (
<TextField
label="URL"
value={question.url}
onValue={action((value: string) => {
question.url = value
})}
placeholder="e.g. https://ourworldindata.org/coronavirus"
helpText="Page or section of a page where the answer to the previous question can be found."
errorMessage={getErrorMessageRelatedQuestionUrl(
question
)}
/>
)}
<Button
onClick={() =>
this.onRemoveRelatedQuestion(idx)
<div>
<TextField
label="Related question"
value={grapher.relatedQuestion?.text}
onValue={action((value: string) => {
if (grapher.relatedQuestion) {
grapher.relatedQuestion.text = value
} else {
grapher.relatedQuestion = {
text: value,
url: "",
}
>
<FontAwesomeIcon icon={faMinus} /> Remove
related question
</Button>
</div>
)
)}
{!relatedQuestions.length && (
<Button onClick={this.onAddRelatedQuestion}>
<FontAwesomeIcon icon={faPlus} /> Add related
question
</Button>
)}
}
})}
placeholder="e.g. How did countries respond to the pandemic?"
helpText="Short question promoting exploration of related content"
softCharacterLimit={50}
/>
{grapher.relatedQuestion?.text && (
<TextField
label="URL"
value={grapher.relatedQuestion.url}
onValue={action((value: string) => {
grapher.relatedQuestion!.url = value
})}
placeholder="e.g. https://ourworldindata.org/coronavirus"
helpText="Page or section of a page where the answer to the previous question can be found."
errorMessage={getErrorMessageRelatedQuestionUrl(
grapher.relatedQuestion
)}
/>
)}
{grapher.relatedQuestion?.text && (
<Button
onClick={() =>
(grapher.relatedQuestion = undefined)
}
>
<FontAwesomeIcon icon={faMinus} /> Remove
related question
</Button>
)}
</div>
</Section>
<Section name="Misc">
<BindString
Expand Down
17 changes: 4 additions & 13 deletions adminSiteServer/apiRouter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ import {
} from "@ourworldindata/types"
import { uuidv7 } from "uuidv7"
import {
defaultGrapherConfig,
migrateGrapherConfigToLatestVersion,
getVariableDataRoute,
getVariableMetadataRoute,
Expand Down Expand Up @@ -328,14 +327,10 @@ const saveNewChart = async (
const parent = shouldInherit
? await getParentByChartConfig(knex, config)
: undefined
const fullParentConfig = mergeGrapherConfigs(
defaultGrapherConfig,
parent?.config ?? {}
)

// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, fullParentConfig)
const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig)
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

// insert patch & full configs into the chart_configs table
Expand Down Expand Up @@ -424,14 +419,10 @@ const updateExistingChart = async (
const parent = shouldInherit
? await getParentByChartConfig(knex, config)
: undefined
const fullParentConfig = mergeGrapherConfigs(
defaultGrapherConfig,
parent?.config ?? {}
)

// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, fullParentConfig)
const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig)
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

const chartConfigId = await db.knexRawFirst<Pick<DbPlainChart, "configId">>(
Expand Down
Loading