Skip to content

Commit

Permalink
馃 backported "Fix column scaling ("Multiply by a number") bugs" (#43690)
Browse files Browse the repository at this point in the history
* Fix column scaling ("Multiply by a number") bugs (#43668)

* return empty string so non-empty labels do not get hidden by empty labels

* fix function name

* apply scaling in transformed dataset

* add missing option to OptionsType

* fix double scaling

* add column scaling to waterfall transformed dataset

* add column value scaling to dataset transform

* fix stacked bar segment labels not responding to column scaling

* fix tooltip values displayed

* since scaling as added to transformedDataset, use transformedDataset for formatters

* revert changes to display percentage instead of absolute value

* fix stacked data labels

* add loki variant for custom y-axis range with column scaling

* fix stack segmented label formatters compact calculation

* scale original dataset and use that instead of modifying transformedDataset

* update loki snapshots

* fix scatter plot dataset scaling

* fix tooltip values being double scaled

* add E2E test to validate behavior

* add helper for not duplicating the column scaling

* port multiple y-axis metrics with column scaling fixes to RowChart

* add waterfall and scatterplot loki variants

* fix type errors and failing unit tests

* fix failing unit tests

* update loki snapshots

* update loki snapshots

---------

Co-authored-by: Jesse Devaney <22608765+JesseSDevaney@users.noreply.github.com>
  • Loading branch information
1 parent e236774 commit a1e1e60
Show file tree
Hide file tree
Showing 56 changed files with 1,313 additions and 49 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified .loki/reference/chrome_laptop_static_viz_ScatterPlot_Default.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified .loki/reference/chrome_laptop_static_viz_ScatterPlot_Goal_Line.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
62 changes: 62 additions & 0 deletions e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
chartPathWithFillColor,
echartsContainer,
getValueLabels,
createQuestion,
chartPathsWithFillColors,
} from "e2e/support/helpers";

const { ORDERS, ORDERS_ID, PEOPLE, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
Expand Down Expand Up @@ -561,4 +563,64 @@ describe("scenarios > visualizations > bar chart", () => {
cy.get(".axis.yr").should("not.exist");
});
});

it("should correctly handle bar sizes and tool-tips for multiple y-axis metrics with column scaling (#43536)", () => {
cy.signInAsAdmin();

const column_settings = { '["name","sum"]': { scale: 0.5 } };
const multiMetric = {
name: "Should split",
type: "query",
query: {
"source-table": ORDERS_ID,
aggregation: [
["sum", ["field", ORDERS.TOTAL]],
["sum", ["field", ORDERS.TOTAL]],
],
breakout: [["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }]],
},
display: "bar",
visualization_settings: {
column_settings,
"graph.show_values": true,
"graph.stackable.stack_type": "stacked",
series_settings: {
sum_2: {
axis: "left",
},
sum: {
axis: "left",
},
},
},
};

createQuestion(multiMetric, { visitQuestion: true });

const [firstMetric, secondMetric] = chartPathsWithFillColors([
"#88BF4D",
"#98D9D9",
]);
firstMetric.then($metricOne => {
const { height: heightMetricOne } = $metricOne[0].getBoundingClientRect();
secondMetric.then($metricTwo => {
const { height: heightMetricTwo } =
$metricTwo[0].getBoundingClientRect();

// since the first metric is scaled to be half of the second metric
// the first bar should be half the size of the first bar
// within a given tolerance
expect(heightMetricOne - heightMetricTwo / 2).to.be.lessThan(0.1);
});
});

chartPathWithFillColor("#88BF4D").first().realHover();
popover().within(() => {
cy.contains("Sum of Total");
// half of the unscaled metric
cy.contains("21,078.43");
// full value of the unscale metric
cy.contains("42,156.87");
});
});
});
1 change: 1 addition & 0 deletions frontend/src/metabase/lib/formatting/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface OptionsType extends TimeOnlyOptions {
removeDay?: boolean;
removeYear?: boolean;
rich?: boolean;
scale?: number;
show_mini_bar?: boolean;
suffix?: string;
type?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ LineCustomYAxisRangeEqualsExtents.args = {
renderingContext,
};

export const CustomYAxisRangeWithColumnScaling = Template.bind({});
CustomYAxisRangeWithColumnScaling.args = {
rawSeries: data.customYAxisRangeWithColumnScaling as any,
dashcardSettings: {},
renderingContext,
};

export const LineFullyNullDimension37902 = Template.bind({});
LineFullyNullDimension37902.args = {
rawSeries: data.lineFullyNullDimension37902 as any,
Expand Down
Loading

0 comments on commit a1e1e60

Please sign in to comment.