Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Sep 4, 2024
1 parent aed8179 commit c37bcf8
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 23 deletions.
17 changes: 10 additions & 7 deletions adminSiteClient/ChartEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,12 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {
this.grapher.object
)

// fetch the new parent config if the indicator has changed
// no-op if the parent indicator hasn't changed
if (currentParentIndicatorId === newParentIndicatorId) return

// fetch the new parent config
let newParentConfig: GrapherInterface | undefined
if (
newParentIndicatorId &&
(currentParentIndicatorId === undefined ||
newParentIndicatorId !== currentParentIndicatorId)
) {
if (newParentIndicatorId) {
newParentConfig = await fetchMergedGrapherConfigByVariableId(
this.manager.admin,
newParentIndicatorId
Expand Down Expand Up @@ -181,8 +180,12 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {
// Need to open intermediary tab before AJAX to avoid popup blockers
const w = window.open("/", "_blank") as Window

// it only makes sense to enable inheritance if the chart has a parent
const shouldEnableInheritance =
!!this.parentVariableId && this.isInheritanceEnabled

const query = new URLSearchParams({
inheritance: this.isInheritanceEnabled ? "enable" : "disable",
inheritance: shouldEnableInheritance ? "enable" : "disable",
})
const targetUrl = `/api/charts?${query}`

Expand Down
4 changes: 0 additions & 4 deletions adminSiteClient/ChartEditorView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,6 @@ export class ChartEditorView<
}}
>
{capitalize(tab)}
{tab === "inheritance" &&
chartEditor &&
chartEditor.isInheritanceEnabled &&
" (enabled)"}
{tab === "refs" &&
chartEditor?.references
? ` (${getFullReferencesCount(
Expand Down
12 changes: 12 additions & 0 deletions adminSiteClient/EditorBasicTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,13 @@ export class EditorBasicTab<
database: EditorDatabase
errorMessagesForDimensions: ErrorMessagesForDimensions
}> {
@action.bound private updateParentConfig() {
const { editor } = this.props
if (isChartEditorInstance(editor)) {
void editor.updateParentConfig()
}
}

@action.bound onChartTypeChange(value: string) {
const { grapher } = this.props.editor
grapher.type = value as ChartTypeName
Expand Down Expand Up @@ -378,6 +385,11 @@ export class EditorBasicTab<
property: DimensionProperty.size,
})
}

// since the parent config depends on the chart type
// (scatters don't have a parent), we might need to update
// the parent config when the type changes
this.updateParentConfig()
}

render() {
Expand Down
17 changes: 9 additions & 8 deletions adminSiteClient/EditorDebugTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,25 @@ class EditorDebugTabForChart extends React.Component<{
})
}

@action.bound onToggleInheritance(newValue: boolean) {
@action.bound onToggleInheritance(shouldBeEnabled: boolean) {
const { patchConfig, parentConfig } = this.props.editor

// update live grapher
const newParentConfig = newValue ? parentConfig : undefined
const newParentConfig = shouldBeEnabled ? parentConfig : undefined
const newConfig = mergeGrapherConfigs(
newParentConfig ?? {},
patchConfig
)
this.props.editor.updateLiveGrapher(newConfig)

this.props.editor.isInheritanceEnabled = newValue
this.props.editor.isInheritanceEnabled = shouldBeEnabled
}

render() {
const {
patchConfig,
parentConfig,
isInheritanceEnabled = false,
isInheritanceEnabled,
fullConfig,
parentVariableId,
grapher,
Expand Down Expand Up @@ -134,13 +134,14 @@ class EditorDebugTabForChart extends React.Component<{
) : (
<p>
This chart may inherit chart settings from
the indicator {variableLink}. Toggle the
option below to enable inheritance.
the indicator {variableLink}, but
inheritance is currently disabled. Toggle
the option below to enable inheritance.
</p>
)}
<Toggle
label="Enable inheritance"
value={isInheritanceEnabled}
value={!!isInheritanceEnabled}
onValue={this.onToggleInheritance}
/>
</Section>
Expand All @@ -156,7 +157,7 @@ class EditorDebugTabForChart extends React.Component<{
rows={7}
readOnly
className="form-control"
value={YAML.stringify(parentConfig ?? {})}
value={YAML.stringify(parentConfig)}
/>
<p className="mt-2">
<a
Expand Down
2 changes: 1 addition & 1 deletion adminSiteClient/IndicatorChartEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface Chart {
export interface IndicatorChartEditorManager
extends AbstractChartEditorManager {
variableId: number
isNewGrapher: boolean
isNewGrapher?: boolean
charts: Chart[]
}

Expand Down
8 changes: 5 additions & 3 deletions adminSiteClient/IndicatorChartEditorPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,23 @@ export class IndicatorChartEditorPage

charts: Chart[] = []

isNewGrapher = false
isInheritanceEnabled = true
isNewGrapher: boolean | undefined = undefined
isInheritanceEnabled: boolean | undefined = undefined

async fetchGrapherConfig(): Promise<void> {
const { variableId } = this
const config = await this.context.admin.getJSON(
`/api/variables/grapherConfigAdmin/${variableId}.patchConfig.json`
)
if (isEmpty(config)) {
this.isNewGrapher = true
this.patchConfig = {
$schema: defaultGrapherConfig.$schema,
dimensions: [{ variableId, property: DimensionProperty.y }],
}
this.isNewGrapher = true
} else {
this.patchConfig = config
this.isNewGrapher = false
}
}

Expand All @@ -55,6 +56,7 @@ export class IndicatorChartEditorPage
`/api/variables/grapherConfigETL/${variableId}.patchConfig.json`
)
this.parentConfig = isEmpty(etlConfig) ? undefined : etlConfig
this.isInheritanceEnabled = true
}

async fetchCharts(): Promise<void> {
Expand Down

0 comments on commit c37bcf8

Please sign in to comment.