Skip to content

Commit

Permalink
[REF] charts: rename copyForSheetId method
Browse files Browse the repository at this point in the history
The difference between both methods `copyForSheetId` and `copyInSheetId`
has always been confusing to me.

This commit attemps to make it clearer by renaming one of the method.

All related helper functions are also renamed accordingly.

closes #5402

Task: 4447450
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
LucasLefevre committed Jan 8, 2025
1 parent 5482a21 commit ea553ef
Show file tree
Hide file tree
Showing 17 changed files with 137 additions and 93 deletions.
6 changes: 3 additions & 3 deletions src/helpers/figures/charts/abstract_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ export abstract class AbstractChart {
abstract updateRanges(applyChange: ApplyRangeChange): AbstractChart;

/**
* Get a copy a the chart adapted to the given sheetId.
* The ranges that are in the same sheet as the chart will be adapted to the given sheetId.
* Duplicate the chart when a sheet is duplicated.
* The ranges that are in the same sheet as the chart are adapted to the new sheetId.
*/
abstract copyForSheetId(sheetId: UID): AbstractChart;
abstract duplicateInDuplicatedSheet(newSheetId: UID): AbstractChart;

/**
* Get a copy a the chart in the given sheetId.
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/bar_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
getDefinedAxis,
shouldRemoveFirstLabel,
toExcelDataset,
Expand Down Expand Up @@ -134,11 +134,15 @@ export class BarChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): BarChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new BarChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): BarChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new BarChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): BarChart {
Expand Down
16 changes: 8 additions & 8 deletions src/helpers/figures/charts/chart_common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { CellErrorType } from "../../../types/errors";
import { ColorGenerator, relativeLuminance } from "../../color";
import { formatValue } from "../../format/format";
import { isDefined, largeMax } from "../../misc";
import { copyRangeWithNewSheetId } from "../../range";
import { duplicateRangeInDuplicatedSheet } from "../../range";
import { rangeReference } from "../../references";
import { getZoneArea, isFullRow, toUnboundedZone, zoneToDimension, zoneToXc } from "../../zones";

Expand Down Expand Up @@ -96,34 +96,34 @@ export function updateChartRangesWithDataSets(
}

/**
* Copy the dataSets given. All the ranges which are on sheetIdFrom will target
* Duplicate the dataSets. All ranges on sheetIdFrom are adapted to target
* sheetIdTo.
*/
export function copyDataSetsWithNewSheetId(
export function duplicateDataSetsInDuplicatedSheet(
sheetIdFrom: UID,
sheetIdTo: UID,
dataSets: DataSet[]
): DataSet[] {
return dataSets.map((ds) => {
return {
dataRange: copyRangeWithNewSheetId(sheetIdFrom, sheetIdTo, ds.dataRange),
dataRange: duplicateRangeInDuplicatedSheet(sheetIdFrom, sheetIdTo, ds.dataRange),
labelCell: ds.labelCell
? copyRangeWithNewSheetId(sheetIdFrom, sheetIdTo, ds.labelCell)
? duplicateRangeInDuplicatedSheet(sheetIdFrom, sheetIdTo, ds.labelCell)
: undefined,
};
});
}

/**
* Copy a range. If the range is on the sheetIdFrom, the range will target
* Duplicate a range. If the range is on the sheetIdFrom, the range will target
* sheetIdTo.
*/
export function copyLabelRangeWithNewSheetId(
export function duplicateLabelRangeInDuplicatedSheet(
sheetIdFrom: UID,
sheetIdTo: UID,
range?: Range
): Range | undefined {
return range ? copyRangeWithNewSheetId(sheetIdFrom, sheetIdTo, range) : undefined;
return range ? duplicateRangeInDuplicatedSheet(sheetIdFrom, sheetIdTo, range) : undefined;
}

/**
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/combo_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
getDefinedAxis,
shouldRemoveFirstLabel,
toExcelDataset,
Expand Down Expand Up @@ -207,11 +207,15 @@ export class ComboChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): ComboChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new ComboChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): ComboChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new ComboChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): ComboChart {
Expand Down
14 changes: 9 additions & 5 deletions src/helpers/figures/charts/gauge_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { createValidRange } from "../../range";
import { rangeReference } from "../../references";
import { toUnboundedZone, zoneToXc } from "../../zones";
import { AbstractChart } from "./abstract_chart";
import { adaptChartRange, copyLabelRangeWithNewSheetId } from "./chart_common";
import { adaptChartRange, duplicateLabelRangeInDuplicatedSheet } from "./chart_common";

type RangeLimitsValidation = (rangeLimit: string, rangeLimitName: string) => CommandResult;
type InflectionPointValueValidation = (
Expand Down Expand Up @@ -204,10 +204,14 @@ export class GaugeChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): GaugeChart {
const dataRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.dataRange);
const definition = this.getDefinitionWithSpecificRanges(dataRange, sheetId);
return new GaugeChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): GaugeChart {
const dataRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.dataRange
);
const definition = this.getDefinitionWithSpecificRanges(dataRange, newSheetId);
return new GaugeChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): GaugeChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/geo_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import { AbstractChart } from "./abstract_chart";
import {
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
transformChartDefinitionWithDataSetsWithZone,
updateChartRangesWithDataSets,
} from "./chart_common";
Expand Down Expand Up @@ -120,11 +120,15 @@ export class GeoChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): GeoChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new GeoChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): GeoChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new GeoChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): GeoChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/line_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
getDefinedAxis,
shouldRemoveFirstLabel,
toExcelDataset,
Expand Down Expand Up @@ -214,11 +214,15 @@ export class LineChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): LineChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new LineChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): LineChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new LineChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): LineChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/pie_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
shouldRemoveFirstLabel,
toExcelDataset,
toExcelLabelRange,
Expand Down Expand Up @@ -144,11 +144,15 @@ export class PieChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): PieChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new PieChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): PieChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new PieChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): PieChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/pyramid_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import { AbstractChart } from "./abstract_chart";
import {
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
transformChartDefinitionWithDataSetsWithZone,
updateChartRangesWithDataSets,
} from "./chart_common";
Expand Down Expand Up @@ -125,11 +125,15 @@ export class PyramidChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): PyramidChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new PyramidChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): PyramidChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new PyramidChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): PyramidChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/radar_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
shouldRemoveFirstLabel,
toExcelDataset,
toExcelLabelRange,
Expand Down Expand Up @@ -130,11 +130,15 @@ export class RadarChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): RadarChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new RadarChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): RadarChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new RadarChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): RadarChart {
Expand Down
18 changes: 11 additions & 7 deletions src/helpers/figures/charts/scatter_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import {
chartFontColor,
checkDataset,
checkLabelRange,
copyDataSetsWithNewSheetId,
copyLabelRangeWithNewSheetId,
createDataSets,
duplicateDataSetsInDuplicatedSheet,
duplicateLabelRangeInDuplicatedSheet,
getDefinedAxis,
shouldRemoveFirstLabel,
toExcelDataset,
Expand Down Expand Up @@ -202,11 +202,15 @@ export class ScatterChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): ScatterChart {
const dataSets = copyDataSetsWithNewSheetId(this.sheetId, sheetId, this.dataSets);
const labelRange = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.labelRange);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, sheetId);
return new ScatterChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): ScatterChart {
const dataSets = duplicateDataSetsInDuplicatedSheet(this.sheetId, newSheetId, this.dataSets);
const labelRange = duplicateLabelRangeInDuplicatedSheet(
this.sheetId,
newSheetId,
this.labelRange
);
const definition = this.getDefinitionWithSpecificDataSets(dataSets, labelRange, newSheetId);
return new ScatterChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): ScatterChart {
Expand Down
12 changes: 6 additions & 6 deletions src/helpers/figures/charts/scorecard_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { rangeReference } from "../../references";
import { clipTextWithEllipsis, drawDecoratedText } from "../../text_helper";
import { toUnboundedZone, zoneToXc } from "../../zones";
import { AbstractChart } from "./abstract_chart";
import { adaptChartRange, copyLabelRangeWithNewSheetId } from "./chart_common";
import { adaptChartRange, duplicateLabelRangeInDuplicatedSheet } from "./chart_common";
import { ScorecardChartConfig } from "./scorecard_chart_config_builder";

function getBaselineText(
Expand Down Expand Up @@ -225,11 +225,11 @@ export class ScorecardChart extends AbstractChart {
};
}

copyForSheetId(sheetId: UID): ScorecardChart {
const baseline = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.baseline);
const keyValue = copyLabelRangeWithNewSheetId(this.sheetId, sheetId, this.keyValue);
const definition = this.getDefinitionWithSpecificRanges(baseline, keyValue, sheetId);
return new ScorecardChart(definition, sheetId, this.getters);
duplicateInDuplicatedSheet(newSheetId: UID): ScorecardChart {
const baseline = duplicateLabelRangeInDuplicatedSheet(this.sheetId, newSheetId, this.baseline);
const keyValue = duplicateLabelRangeInDuplicatedSheet(this.sheetId, newSheetId, this.keyValue);
const definition = this.getDefinitionWithSpecificRanges(baseline, keyValue, newSheetId);
return new ScorecardChart(definition, newSheetId, this.getters);
}

copyInSheetId(sheetId: UID): ScorecardChart {
Expand Down
Loading

0 comments on commit ea553ef

Please sign in to comment.