Fix dimension legend with multiple series

vizzuhq · Nov 5, 2024 · 2fdc5de · 2fdc5de
1 parent 21117a0
commit 2fdc5de
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@
   - From now, it is prohibited to set only aggregator without series name.
   - Fix series parsing when aggregator comes first.
   - Fix disappearing dimension when aggregated dimension was already set. 
+- Fix legend with multiple dimension duplicated markers:
+  - Markers of color are never merged.
+  - Markers of size are always merged.
+  - Markers of lightness are only merged when labelLevel == 0.
 
 ## [0.15.0] - 2024-10-28
 

diff --git a/src/chart/generator/axis.h b/src/chart/generator/axis.h
@@ -125,7 +125,7 @@ struct DimensionAxis
 	bool add(const Data::SliceIndex &index,
 	    double value,
 	    const Math::Range<double> &range,
-	    bool merge = true);
+	    bool merge);
 	[[nodiscard]] bool operator==(const DimensionAxis &other) const;
 
 	[[nodiscard]] auto begin()

diff --git a/src/chart/generator/plotbuilder.cpp b/src/chart/generator/plotbuilder.cpp
@@ -394,8 +394,8 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable, T type)
 	}
 	else {
 		constexpr bool isTypeAxis = std::is_same_v<T, AxisId>;
-		if constexpr (isTypeAxis) {
-			auto merge = scale.labelLevel == 0;
+		if constexpr (auto merge = scale.labelLevel == 0;
+		              isTypeAxis) {
 			for (const auto &marker : plot->markers) {
 				if (!marker.enabled) continue;
 
@@ -421,7 +421,10 @@ void PlotBuilder::calcAxis(const Data::DataTable &dataTable, T type)
 				    sliceIndex
 				    && axis.dimension.add(*sliceIndex,
 				        i,
-				        {count, count}))
+				        {count, count},
+				        type == LegendId::size
+				            || (type == LegendId::lightness
+				                && merge)))
 					count += 1;
 		}
 		auto hasLabel = axis.dimension.setLabels(

diff --git a/src/chart/options/autoparam.h b/src/chart/options/autoparam.h
@@ -76,6 +76,8 @@ template <typename Type, bool nullable = false> struct AutoParam
 
 	bool operator==(const AutoParam &other) const = default;
 
+	const std::optional<Type> &getValueOrAuto() { return value; }
+
 private:
 	bool autoSet{};
 	std::optional<Type> value;

diff --git a/src/chart/options/channel.cpp b/src/chart/options/channel.cpp
@@ -154,16 +154,15 @@ bool Channel::isSeriesUsed(const Data::SeriesIndex &index) const
 
 void Channel::reset()
 {
-	set.measureId = std::nullopt;
-	set.dimensionIds.clear();
-	title = Base::AutoParam<std::string, true>{};
+	set = {};
+	labelLevel = {};
+	title = {};
 	axis = Base::AutoBool();
 	labels = Base::AutoBool();
 	ticks = Base::AutoBool();
-	interlacing = Base::AutoBool();
 	guides = Base::AutoBool();
 	markerGuides = Base::AutoBool();
-	labelLevel = 0;
+	interlacing = Base::AutoBool();
 }
 
 bool Channel::isEmpty() const

diff --git a/src/chart/rendering/drawlegend.cpp b/src/chart/rendering/drawlegend.cpp
@@ -336,7 +336,7 @@ Math::Range<double> DrawLegend::markersLegendRange(const Info &info)
 	if (info.dimensionEnabled)
 		for (const auto &item : info.dimension)
 			res.include({item.range.getMin() * info.itemHeight,
-			    (item.range.getMax() + 1) * info.itemHeight});
+			    (item.range.getMin() + 1) * info.itemHeight});
 
 	return res;
 }

diff --git a/test/e2e/test_cases/test_cases.json b/test/e2e/test_cases/test_cases.json
@@ -56,7 +56,7 @@
 			"refs": ["10761b9"]
 		},
 		"basic_animations/legend_transitions/color_2discrete_anim": {
-			"refs": ["9c5a5fb"]
+			"refs": ["4fb1ac9"]
 		},
 		"basic_animations/legend_transitions/color_conti_anim": {
 			"refs": ["64caf45"]
@@ -299,7 +299,7 @@
 			"refs": ["6c42aad"]
 		},
 		"operations/orientation_tutorial_data/line_orientation": {
-			"refs": ["c6ddbab"]
+			"refs": ["1777877"]
 		},
 		"operations/orientation_tutorial_data/rectangle_orientation": {
 			"refs": ["4c39423"]