From dd6f0cc45346b2d8ad49cb9fc34acfddac68bb01 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 30 Aug 2021 19:10:35 +0300
Subject: [PATCH] Vertical legend fixes for LineAreaBarChart (#17552)
---
.../components/VisualizationResult.jsx | 1 +
.../visualizations/components/LineAreaBarChart.jsx | 5 +++++
.../components/LineAreaBarChart.styled.jsx | 3 ++-
.../visualizations/components/Visualization.jsx | 2 ++
.../components/legend/Legend.styled.jsx | 1 +
.../components/legend/LegendCaption.jsx | 5 ++++-
.../visualizations/components/legend/LegendItem.jsx | 2 +-
.../components/legend/LegendLayout.jsx | 13 +++++++++++--
.../components/legend/LegendLayout.styled.jsx | 3 ++-
.../drillthroughs/chart_drill.cy.spec.js | 6 +++---
10 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
index a1c1c2ed5c1..ba4cb4f1a6c 100644
--- a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
+++ b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
@@ -105,6 +105,7 @@ export default class VisualizationResult extends Component {
rawSeries={rawSeries}
onChangeCardAndRun={navigateToNewCardInsideQB}
isEditing={true}
+ isQueryBuilder={true}
showTitle={false}
metadata={question.metadata()}
onOpenChartSettings={this.props.onOpenChartSettings}
diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
index 8d12a0c5c2c..563ad39de5b 100644
--- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
+++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
@@ -358,6 +358,8 @@ export default class LineAreaBarChart extends Component {
hovered,
headerIcon,
actionButtons,
+ isFullscreen,
+ isQueryBuilder,
onHoverChange,
onAddSeries,
onRemoveSeries,
@@ -381,6 +383,7 @@ export default class LineAreaBarChart extends Component {
this.getHoverClasses(),
this.props.className,
)}
+ isQueryBuilder={isQueryBuilder}
>
{hasTitle && (
<ChartLegendCaption
@@ -397,6 +400,8 @@ export default class LineAreaBarChart extends Component {
hovered={hovered}
hasLegend={hasLegend}
actionButtons={!hasTitle ? actionButtons : undefined}
+ isFullscreen={isFullscreen}
+ isQueryBuilder={isQueryBuilder}
onHoverChange={onHoverChange}
onAddSeries={!hasBreakout ? onAddSeries : undefined}
onRemoveSeries={!hasBreakout ? onRemoveSeries : undefined}
diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.styled.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.styled.jsx
index 715ba97822f..ab55ed7a636 100644
--- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.styled.jsx
+++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.styled.jsx
@@ -4,7 +4,8 @@ import LegendCaption from "./legend/LegendCaption";
export const LineAreaBarChartRoot = styled.div`
display: flex;
flex-direction: column;
- padding: 0.5rem 1rem;
+ padding: ${({ isQueryBuilder }) =>
+ isQueryBuilder ? "1rem 1rem 1rem 2rem" : "0.5rem 1rem"};
overflow: hidden;
`;
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index e87baeea71e..ef75b79850d 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -63,6 +63,7 @@ type Props = {
isDashboard: boolean,
isEditing: boolean,
isSettings: boolean,
+ isQueryBuilder: boolean,
headerIcon?: {
name: string,
@@ -161,6 +162,7 @@ export default class Visualization extends React.PureComponent {
isDashboard: false,
isEditing: false,
isSettings: false,
+ isQueryBuilder: false,
onUpdateVisualizationSettings: () => {},
// prefer passing in a function that doesn't cause the application to reload
onChangeLocation: location => {
diff --git a/frontend/src/metabase/visualizations/components/legend/Legend.styled.jsx b/frontend/src/metabase/visualizations/components/legend/Legend.styled.jsx
index 859507b5564..149267a179a 100644
--- a/frontend/src/metabase/visualizations/components/legend/Legend.styled.jsx
+++ b/frontend/src/metabase/visualizations/components/legend/Legend.styled.jsx
@@ -4,6 +4,7 @@ import colors, { darken } from "metabase/lib/colors";
export const LegendRoot = styled.div`
display: flex;
flex-direction: ${({ isVertical }) => (isVertical ? "column" : "row")};
+ overflow: ${({ isVertical }) => (isVertical ? "" : "hidden")};
`;
export const LegendLink = styled.div`
diff --git a/frontend/src/metabase/visualizations/components/legend/LegendCaption.jsx b/frontend/src/metabase/visualizations/components/legend/LegendCaption.jsx
index c54c32f8682..215f247290f 100644
--- a/frontend/src/metabase/visualizations/components/legend/LegendCaption.jsx
+++ b/frontend/src/metabase/visualizations/components/legend/LegendCaption.jsx
@@ -31,7 +31,10 @@ const LegendCaption = ({
return (
<LegendCaptionRoot className={className} data-testid="legend-caption">
{icon && <LegendLabelIcon {...icon} />}
- <LegendLabel onClick={onSelectTitle}>
+ <LegendLabel
+ className="fullscreen-normal-text fullscreen-night-text"
+ onClick={onSelectTitle}
+ >
<Ellipsified>{title}</Ellipsified>
</LegendLabel>
{description && (
diff --git a/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx b/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx
index abd52a44e58..d88b4f64451 100644
--- a/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx
+++ b/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx
@@ -55,7 +55,7 @@ const LegendItem = ({
onMouseLeave={onHoverChange && handleItemMouseLeave}
>
<LegendItemDot color={color} />
- <LegendItemTitle>
+ <LegendItemTitle className="fullscreen-normal-text fullscreen-night-text">
<Ellipsified>{label}</Ellipsified>
</LegendItemTitle>
</LegendItemLabel>
diff --git a/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx b/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx
index 6570347532b..b04c60f2a9d 100644
--- a/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx
+++ b/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx
@@ -13,6 +13,7 @@ import {
const MIN_ITEM_WIDTH = 100;
const MIN_ITEM_HEIGHT = 25;
+const MIN_ITEM_HEIGHT_LARGE = 31;
const MIN_LEGEND_WIDTH = 400;
const propTypes = {
@@ -24,6 +25,8 @@ const propTypes = {
height: PropTypes.number,
hasLegend: PropTypes.bool,
actionButtons: PropTypes.node,
+ isFullscreen: PropTypes.bool,
+ isQueryBuilder: PropTypes.bool,
children: PropTypes.node,
onHoverChange: PropTypes.func,
onAddSeries: PropTypes.func,
@@ -40,14 +43,17 @@ const LegendLayout = ({
height = 0,
hasLegend,
actionButtons,
+ isFullscreen,
+ isQueryBuilder,
children,
onHoverChange,
onAddSeries,
onSelectSeries,
onRemoveSeries,
}) => {
+ const itemHeight = !isFullscreen ? MIN_ITEM_HEIGHT : MIN_ITEM_HEIGHT_LARGE;
const maxXItems = Math.floor(width / MIN_ITEM_WIDTH);
- const maxYItems = Math.floor(height / MIN_ITEM_HEIGHT);
+ const maxYItems = Math.floor(height / itemHeight);
const maxYLabels = Math.max(maxYItems - 1, 0);
const minYLabels = labels.length > maxYItems ? maxYLabels : labels.length;
@@ -59,7 +65,10 @@ const LegendLayout = ({
return (
<LegendLayoutRoot className={className} isVertical={isVertical}>
{isVisible && (
- <LegendContainer isVertical={isVertical}>
+ <LegendContainer
+ isVertical={isVertical}
+ isQueryBuilder={isQueryBuilder}
+ >
<Legend
labels={labels}
colors={colors}
diff --git a/frontend/src/metabase/visualizations/components/legend/LegendLayout.styled.jsx b/frontend/src/metabase/visualizations/components/legend/LegendLayout.styled.jsx
index 64e4448323c..6d5a87fc982 100644
--- a/frontend/src/metabase/visualizations/components/legend/LegendLayout.styled.jsx
+++ b/frontend/src/metabase/visualizations/components/legend/LegendLayout.styled.jsx
@@ -17,7 +17,8 @@ export const LegendContainer = styled.div`
display: ${({ isVertical }) => (isVertical ? "block" : "flex")};
max-width: ${({ isVertical }) => (isVertical ? "25%" : "")};
max-width: ${({ isVertical }) => (isVertical ? "min(25%, 20rem)" : "")};
- margin-right: ${({ isVertical }) => (isVertical ? "0.5rem" : "")};
+ margin-right: ${({ isVertical, isQueryBuilder }) =>
+ isVertical ? (isQueryBuilder ? "2.5rem" : "0.5rem") : ""};
margin-bottom: ${({ isVertical }) => (isVertical ? "" : "0.5rem")};
`;
diff --git a/frontend/test/metabase/scenarios/visualizations/drillthroughs/chart_drill.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/drillthroughs/chart_drill.cy.spec.js
index 58273f3a4ab..a1feb0b0822 100644
--- a/frontend/test/metabase/scenarios/visualizations/drillthroughs/chart_drill.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/drillthroughs/chart_drill.cy.spec.js
@@ -52,9 +52,9 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
// drag across to filter
cy.get(".Visualization")
- .trigger("mousedown", 100, 200)
- .trigger("mousemove", 210, 200)
- .trigger("mouseup", 210, 200);
+ .trigger("mousedown", 120, 200)
+ .trigger("mousemove", 230, 200)
+ .trigger("mouseup", 230, 200);
// new filter applied
// Note: Test was flaking because apparently mouseup doesn't always happen at the same position.
--
GitLab