Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(barSeries): error rendering bars with negative log scale #2407

Merged
merged 21 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
30f5861
fix(barSeries): error rendering bars with negative log scale
nickofthyme Apr 11, 2024
bed2b45
fix: banded tooltip for barSeries specs
nickofthyme Apr 11, 2024
464b807
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Apr 11, 2024
b886207
Merge branch 'main' into fix-neg-log
nickofthyme Apr 25, 2024
6d3e0f3
fix: render highlighter and geom matcher
nickofthyme Apr 26, 2024
d51ac12
fix: minBarHeight issue
nickofthyme Apr 26, 2024
b3b1d36
fix: pre-sort tooltip values to prevent swapping when cursor moves
nickofthyme Apr 26, 2024
4381347
Merge remote-tracking branch 'origin/fix-neg-log' into fix-neg-log
nickofthyme Apr 26, 2024
ddd3d99
chore: remove old updated screenshots
nickofthyme Apr 26, 2024
bf89632
fix: y0 on bars where min height it applied
nickofthyme Apr 26, 2024
a26581f
Merge branch 'main' into fix-neg-log
nickofthyme Apr 26, 2024
a895568
chore: update vrt test urls
nickofthyme Apr 26, 2024
63e8136
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] Apr 26, 2024
7bda378
test: fix test cases
nickofthyme Apr 28, 2024
b10dcf5
fix: reversed and flipped min height bar geometries and rendering issues
nickofthyme Apr 30, 2024
2baed4f
test: add tests for new bar rendering util changes
nickofthyme Apr 30, 2024
01559c5
refactor: simplified version
markov00 May 6, 2024
342be6d
fix: minHeight for absolute 0 value
nickofthyme May 6, 2024
245dcf3
fix: filter duplicate bar geom on highlighter
nickofthyme May 6, 2024
652d7f8
Merge branch 'main' into fix-neg-log
nickofthyme May 7, 2024
332fc5d
test(vrt): update screenshots [skip ci]
elastic-datavis[bot] May 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
6 changes: 6 additions & 0 deletions e2e/tests/bar_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,10 @@ test.describe('Bar series stories', () => {
);
});
});

test('should not render min bar heights for log scale values at baseline', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/bar-chart--min-height&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Banded=true&knob-Custom No Results message=No Results&knob-Nice y ticks=true&knob-Scale Type=linear&knob-Series Type=bar&knob-Show positive data=true&knob-Split=true&knob-logMinLimit=1&knob-minBarHeight=5&knob-scale=log&knob-scaleType=log',
);
});
});
24 changes: 24 additions & 0 deletions e2e/tests/test_cases_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,28 @@ test.describe('Test cases stories', () => {
);
});
});

test.describe('Log scales', () => {
test('should correctly render negative values from baseline when banded', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--log-with-negative-values&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Show legend=&knob-Scale Type=log&knob-Series Type=bar&knob-logMinLimit=1&knob-Nice y ticks=&knob-Banded=true&knob-Split=&knob-Stacked=false&knob-Show positive data=',
);
});

test('should correctly render tooltip values for banded bars', async ({ page }) => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--log-with-negative-values&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Show legend=&knob-Scale Type=log&knob-Series Type=bar&knob-logMinLimit=1&knob-Nice y ticks=&knob-Banded=true&knob-Split=&knob-Stacked=false&knob-Show positive data=',
{
top: 240,
right: 240,
},
);
});

test('should correctly render negative values from baseline when stacked', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/test-cases--log-with-negative-values&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Show legend=&knob-Scale Type=log&knob-Series Type=bar&knob-logMinLimit=1&knob-Nice y ticks=&knob-Banded=&knob-Split=true&knob-Stacked=true&knob-Show positive data=',
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { InitStatus, getInternalIsInitializedSelector } from '../../../../state/
import { isBrushingSelector } from '../../../../state/selectors/is_brushing';
import { getColorFromVariant, Rotation } from '../../../../utils/common';
import { Dimensions } from '../../../../utils/dimensions';
import { isPointGeometry, IndexedGeometry, PointGeometry } from '../../../../utils/geometry';
import { isPointGeometry, IndexedGeometry, PointGeometry, isBarGeometry } from '../../../../utils/geometry';
import { LIGHT_THEME } from '../../../../utils/themes/light_theme';
import { HighlighterStyle } from '../../../../utils/themes/theme';
import { computeChartDimensionsSelector } from '../../state/selectors/compute_chart_dimensions';
Expand Down Expand Up @@ -57,11 +57,20 @@ class HighlighterComponent extends React.Component<HighlighterProps> {
static displayName = 'Highlighter';

render() {
const { highlightedGeometries, chartDimensions, chartRotation, chartId, zIndex, isBrushing, style } = this.props;
const { chartDimensions, chartRotation, chartId, zIndex, isBrushing, style } = this.props;
if (isBrushing) return null;
const clipWidth = [90, -90].includes(chartRotation) ? chartDimensions.height : chartDimensions.width;
const clipHeight = [90, -90].includes(chartRotation) ? chartDimensions.width : chartDimensions.height;
const clipPathId = `echHighlighterClipPath__${chartId}`;

const seenBarSeries = new Set<string>();
const highlightedGeometries = this.props.highlightedGeometries.filter((geom) => {
if (!isBarGeometry(geom)) return true;
const seen = seenBarSeries.has(geom.seriesIdentifier.key);
seenBarSeries.add(geom.seriesIdentifier.key);
return !seen;
});

return (
<svg className="echHighlighter" style={{ zIndex }}>
<defs>
Expand Down Expand Up @@ -100,11 +109,12 @@ class HighlighterComponent extends React.Component<HighlighterProps> {
</g>
);
}

return (
<rect
key={i}
x={x}
y={y}
y={geom.y}
width={geom.width}
height={geom.height}
transform={geomTransform}
Expand Down
Loading