Skip to content

Commit

Permalink
Merge pull request #621 from vizzuhq/regression_fix
Browse files Browse the repository at this point in the history
0.14.0 Regression - fake-split intermediate step not the same
  • Loading branch information
schaumb authored Nov 28, 2024
2 parents 0398c1c + 2708c4b commit 30b7178
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### Fixed

- Fix align on fake-split charts
- Drilldown on split chart is fade.
- Fix aggregate on split chart.
- Do not interpolate hiding/showing legend
- Fix aggregator interface for 'set' channel parameter:
- From now not accepted the same dimension on the same channel.
Expand Down
2 changes: 1 addition & 1 deletion src/chart/animator/animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void Animation::addKeyframe(const Gen::PlotPtr &next,
auto begin = std::ref(intermediate0 ? intermediate0 : target);

auto &&intermediate1Instant =
intermediate1 && strategy == RegroupStrategy::aggregate
intermediate1 && strategy != RegroupStrategy::fade
&& begin.get()->getOptions()->looksTheSame(
*intermediate1->getOptions());
begin = intermediate1 ? std::ref(intermediate1) : begin;
Expand Down
11 changes: 8 additions & 3 deletions src/chart/options/options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ Channels Options::shadowChannels() const
&ch2 = shadow.at(ChannelId::noop);
auto &&stacker : shadow.getDimensions({data(stackChannels),
std::size_t{1} + secondary.has_value()})) {
ch1.removeSeries(stacker);
if (stackChannelType() != subAxisType() || !isSplit())
ch1.removeSeries(stacker);
ch2.removeSeries(stacker);
}

Expand All @@ -122,7 +123,7 @@ void Options::drilldownTo(const Options &other)
{
auto &stackChannel = this->stackChannel();

if (this->split && !isSplit()) this->split = {};
if (!isSplit() || !other.isSplit()) this->split = {};

for (auto &&dim : other.getChannels().getDimensions())
if (!getChannels().isSeriesUsed(dim))
Expand All @@ -134,12 +135,14 @@ void Options::intersection(const Options &other)
for (auto &&dim : getChannels().getDimensions())
if (!other.getChannels().isSeriesUsed(dim))
getChannels().removeSeries(dim);

split = {};
}

bool Options::looksTheSame(const Options &other) const
{
if (channels.anyAxisSet()
&& channels.at(Gen::ChannelId::label).isEmpty()) {
&& channels.at(ChannelId::label).isEmpty()) {
auto thisCopy = *this;
thisCopy.simplify();

Expand All @@ -154,6 +157,8 @@ bool Options::looksTheSame(const Options &other) const

void Options::simplify()
{
if (isSplit()) return;

// remove all dimensions, only used at the end of stack
auto &stackChannel = this->stackChannel();

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/test_cases/test_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -2453,7 +2453,7 @@
"refs": ["e5678fa"]
},
"ww_noFade/wNoFade_Tests/Marker_transition_problem/area_column_time_sum": {
"refs": ["67ef3d7"]
"refs": ["727c6d5"]
},
"ww_noFade/wNoFade_Tests/Marker_transition_problem/area_orientation": {
"refs": ["8c0d580"]
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/tests/fixes.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"refs": ["1732a49"]
},
"143": {
"refs": ["fb8a740"]
"refs": ["95b9c83"]
},
"144": {
"refs": ["fde02e4"]
Expand Down
51 changes: 46 additions & 5 deletions test/e2e/tests/fixes/143.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ const testSteps = [
type: 'dimension',
values: ['a', 'a', 'a', 'b', 'b', 'b', 'c', 'c', 'c']
},
{
name: 'Letters2',
type: 'dimension',
values: ['a', 'a', 'a', 'a', 'a', 'b', 'b', 'b', 'b']
},
{
name: 'Val',
type: 'measure',
Expand All @@ -24,19 +29,55 @@ const testSteps = [
data,
config: {
y: 'Colors',
x: 'Val'
x: 'Val',
split: true
}
})
},
(chart) =>
chart.animate(
{
y: 'Val',
x: 'Letters',
split: true
y: ['Letters2', 'Val'],
x: 'Letters'
},
{ regroupStrategy: 'drilldown' }
)
),
(chart) =>
chart.animate(
{
y: ['Letters2', 'Colors', 'Val']
},
{ regroupStrategy: 'drilldown' }
),
(chart) =>
chart.animate({
config: {
y: 'Colors',
x: 'Val'
}
}),
(chart) =>
chart.animate({
config: {
y: ['Letters2', 'Val'],
x: 'Letters'
}
}),
(chart) =>
chart.animate({
y: ['Letters2', 'Val']
}),
(chart) =>
chart.animate({
y: ['Letters2', 'Colors', 'Val']
}),
(chart) =>
chart.animate({
config: {
y: ['Letters2', 'Colors'],
x: 'Val'
}
})
]

export default testSteps

0 comments on commit 30b7178

Please sign in to comment.