Skip to content

Commit 8c61320

Browse files
PiTremPiTrem
committed
fix(d3): clear multiplet annotations when the next spectrum has none
drawMtply returned before its data join when the multiplicity stack was empty (or the layout has no multiplicity), so exit().remove() never ran and the bars, peak markers and labels of the previously drawn spectrum stayed on the chart. The early return became effective in 156408c, which corrected `mpys === 0` (never true) to `mpys.length === 0`. Empty the four multiplicity layers before returning, in LineFocus and MultiFocus; MultiFocus's no-selection branch now uses the same helper. Closes #341
1 parent 05b0c0e commit 8c61320

5 files changed

Lines changed: 113 additions & 45 deletions

File tree

‎dist/components/d3_line/line_focus.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,13 @@ class LineFocus {
457457
}
458458
this.drawVisualSplitLines(showIntegSplit ? itgs : [], shift, ignoreRef);
459459
}
460+
clearMtply() {
461+
// Nothing to draw: still empty the layers, or the previous spectrum's multiplets stay drawn.
462+
this.tags.mpybPath.selectAll('path').remove();
463+
this.tags.mpyt1Path.selectAll('text').remove();
464+
this.tags.mpyt2Path.selectAll('text').remove();
465+
this.tags.mpypPath.selectAll('path').remove();
466+
}
460467
drawMtply(mtplySt) {
461468
const {
462469
sameXY,
@@ -477,7 +484,10 @@ class LineFocus {
477484
const hasValidExtent = extent => extent && Number.isFinite(extent.xL) && Number.isFinite(extent.xU);
478485
const mpys = stack.filter(m => hasValidExtent(m?.xExtent));
479486
const isDisable = _cfg.default.btnCmdMpy(this.layout);
480-
if (mpys.length === 0 || isDisable) return;
487+
if (mpys.length === 0 || isDisable) {
488+
this.clearMtply();
489+
return;
490+
}
481491
const activeExtent = hasValidExtent(smExtext) ? smExtext : mpys[0].xExtent;
482492
// rescale for zoom
483493
const {

‎dist/components/d3_multi/multi_focus.js‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,13 @@ class MultiFocus {
710710
}
711711
this.drawVisualSplitLines(showIntegSplit ? itgs : [], shift, ignoreRef);
712712
}
713+
clearMtply() {
714+
// Nothing to draw: still empty the layers, or the previous spectrum's multiplets stay drawn.
715+
this.tags.mpybPath.selectAll('path').remove();
716+
this.tags.mpyt1Path.selectAll('text').remove();
717+
this.tags.mpyt2Path.selectAll('text').remove();
718+
this.tags.mpypPath.selectAll('path').remove();
719+
}
713720
drawMtply(mtplySt) {
714721
const {
715722
sameXY,
@@ -722,25 +729,7 @@ class MultiFocus {
722729
} = mtplySt;
723730
const selectedMulti = multiplicities[this.jcampIdx];
724731
if (selectedMulti === false || selectedMulti === undefined) {
725-
const mpys = [];
726-
const mpyb = this.tags.mpybPath.selectAll('path').data(mpys);
727-
mpyb.exit().attr('class', 'exit').remove();
728-
const mpyt1 = this.tags.mpyt1Path.selectAll('text').data(mpys);
729-
mpyt1.exit().attr('class', 'exit').remove();
730-
const mpyt2 = this.tags.mpyt2Path.selectAll('text').data(mpys);
731-
mpyt2.exit().attr('class', 'exit').remove();
732-
let mPeaks = mpys.map(m => {
733-
const {
734-
peaks,
735-
xExtent
736-
} = m;
737-
return peaks.map(p => Object.assign({}, p, {
738-
xExtent
739-
}));
740-
});
741-
mPeaks = [].concat(...mPeaks);
742-
const mpyp = this.tags.mpypPath.selectAll('path').data(mPeaks);
743-
mpyp.exit().attr('class', 'exit').remove();
732+
this.clearMtply();
744733
return;
745734
}
746735
const {
@@ -751,7 +740,10 @@ class MultiFocus {
751740
const hasValidExtent = extent => extent && Number.isFinite(extent.xL) && Number.isFinite(extent.xU);
752741
const mpys = stack.filter(m => hasValidExtent(m?.xExtent));
753742
const isDisable = _cfg.default.btnCmdMpy(this.layout);
754-
if (mpys.length === 0 || isDisable) return;
743+
if (mpys.length === 0 || isDisable) {
744+
this.clearMtply();
745+
return;
746+
}
755747
const activeExtent = hasValidExtent(smExtext) ? smExtext : mpys[0].xExtent;
756748
// rescale for zoom
757749
const {

‎src/__tests__/units/components/draw_mtply_clear.test.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import * as d3 from 'd3';
2+
import LineFocus from '../../../components/d3_line/line_focus';
3+
import MultiFocus from '../../../components/d3_multi/multi_focus';
4+
import { LIST_LAYOUT } from '../../../constants/list_layout';
5+
6+
// Opening a spectrum without multiplets after one with multiplets must not leave the
7+
// previous multiplet bars, peak markers and labels on the chart (#341).
8+
const buildFocus = (Focus, extra = {}) => {
9+
const svg = d3.select(document.body).append('svg');
10+
const tags = {};
11+
['mpybPath', 'mpyt1Path', 'mpyt2Path', 'mpypPath'].forEach((k) => {
12+
tags[k] = svg.append('g').attr('class', k);
13+
});
14+
const focus = Object.create(Focus.prototype);
15+
return Object.assign(focus, {
16+
svg,
17+
tags,
18+
h: 300,
19+
layout: LIST_LAYOUT.H1,
20+
scales: { x: d3.scaleLinear().domain([10, 0]).range([0, 1000]), y: d3.scaleLinear() },
21+
shouldUpdate: {},
22+
onClickTarget: () => {},
23+
...extra,
24+
});
25+
};
26+
27+
const multiplet = (xL, xU) => ({
28+
xExtent: { xL, xU },
29+
mpyType: 'd',
30+
peaks: [{ x: xL + 0.01, y: 1 }, { x: xU - 0.01, y: 1 }],
31+
});
32+
const stack = [multiplet(7.9, 8.0), multiplet(7.1, 7.2)];
33+
const count = (focus) => ['.mpyb', '.mpyp', '.mpyt1', '.mpyt2']
34+
.reduce((n, sel) => n + focus.svg.selectAll(sel).size(), 0);
35+
36+
describe('drawMtply clears the multiplicity layers when there is nothing to draw', () => {
37+
afterEach(() => { document.body.innerHTML = ''; });
38+
39+
it('LineFocus: a stack, then an empty stack', () => {
40+
const focus = buildFocus(LineFocus);
41+
focus.drawMtply({ selectedIdx: 0, multiplicities: [{ stack, smExtext: false, shift: 0 }] });
42+
expect(focus.svg.selectAll('.mpyt1').size()).toEqual(2);
43+
44+
focus.drawMtply({ selectedIdx: 0, multiplicities: [{ stack: [], smExtext: false, shift: 0 }] });
45+
expect(count(focus)).toEqual(0);
46+
});
47+
48+
it('LineFocus: a stack, then a non-NMR layout (multiplicity disabled)', () => {
49+
const focus = buildFocus(LineFocus);
50+
const mtplySt = { selectedIdx: 0, multiplicities: [{ stack, smExtext: false, shift: 0 }] };
51+
focus.drawMtply(mtplySt);
52+
focus.layout = LIST_LAYOUT.IR;
53+
focus.drawMtply(mtplySt);
54+
expect(count(focus)).toEqual(0);
55+
});
56+
57+
it('MultiFocus: a stack, then an empty stack', () => {
58+
const focus = buildFocus(MultiFocus, { jcampIdx: 0 });
59+
focus.drawMtply({ multiplicities: [{ stack, smExtext: false, shift: 0 }] });
60+
expect(focus.svg.selectAll('.mpyt1').size()).toEqual(2);
61+
62+
focus.drawMtply({ multiplicities: [{ stack: [], smExtext: false, shift: 0 }] });
63+
expect(count(focus)).toEqual(0);
64+
});
65+
});

‎src/components/d3_line/line_focus.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,14 @@ class LineFocus {
549549
this.drawVisualSplitLines(showIntegSplit ? itgs : [], shift, ignoreRef);
550550
}
551551

552+
clearMtply() {
553+
// Nothing to draw: still empty the layers, or the previous spectrum's multiplets stay drawn.
554+
this.tags.mpybPath.selectAll('path').remove();
555+
this.tags.mpyt1Path.selectAll('text').remove();
556+
this.tags.mpyt2Path.selectAll('text').remove();
557+
this.tags.mpypPath.selectAll('path').remove();
558+
}
559+
552560
drawMtply(mtplySt) {
553561
const { sameXY, sameLySt, sameMySt } = this.shouldUpdate;
554562
if (sameXY && sameLySt && sameMySt) return;
@@ -566,7 +574,10 @@ class LineFocus {
566574
);
567575
const mpys = stack.filter((m) => hasValidExtent(m?.xExtent));
568576
const isDisable = Cfg.btnCmdMpy(this.layout);
569-
if (mpys.length === 0 || isDisable) return;
577+
if (mpys.length === 0 || isDisable) {
578+
this.clearMtply();
579+
return;
580+
}
570581
const activeExtent = hasValidExtent(smExtext) ? smExtext : mpys[0].xExtent;
571582
// rescale for zoom
572583
const { xt } = TfRescale(this);

‎src/components/d3_multi/multi_focus.js‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,14 @@ class MultiFocus {
796796
this.drawVisualSplitLines(showIntegSplit ? itgs : [], shift, ignoreRef);
797797
}
798798

799+
clearMtply() {
800+
// Nothing to draw: still empty the layers, or the previous spectrum's multiplets stay drawn.
801+
this.tags.mpybPath.selectAll('path').remove();
802+
this.tags.mpyt1Path.selectAll('text').remove();
803+
this.tags.mpyt2Path.selectAll('text').remove();
804+
this.tags.mpypPath.selectAll('path').remove();
805+
}
806+
799807
drawMtply(mtplySt) {
800808
const { sameXY, sameLySt, sameMySt } = this.shouldUpdate;
801809
if (sameXY && sameLySt && sameMySt) return;
@@ -804,28 +812,7 @@ class MultiFocus {
804812
const selectedMulti = multiplicities[this.jcampIdx];
805813

806814
if (selectedMulti === false || selectedMulti === undefined) {
807-
const mpys = [];
808-
const mpyb = this.tags.mpybPath.selectAll('path').data(mpys);
809-
mpyb.exit()
810-
.attr('class', 'exit')
811-
.remove();
812-
const mpyt1 = this.tags.mpyt1Path.selectAll('text').data(mpys);
813-
mpyt1.exit()
814-
.attr('class', 'exit')
815-
.remove();
816-
const mpyt2 = this.tags.mpyt2Path.selectAll('text').data(mpys);
817-
mpyt2.exit()
818-
.attr('class', 'exit')
819-
.remove();
820-
let mPeaks = mpys.map((m) => {
821-
const { peaks, xExtent } = m;
822-
return peaks.map((p) => Object.assign({}, p, { xExtent }));
823-
});
824-
mPeaks = [].concat(...mPeaks);
825-
const mpyp = this.tags.mpypPath.selectAll('path').data(mPeaks);
826-
mpyp.exit()
827-
.attr('class', 'exit')
828-
.remove();
815+
this.clearMtply();
829816
return;
830817
}
831818

@@ -839,7 +826,10 @@ class MultiFocus {
839826
);
840827
const mpys = stack.filter((m) => hasValidExtent(m?.xExtent));
841828
const isDisable = Cfg.btnCmdMpy(this.layout);
842-
if (mpys.length === 0 || isDisable) return;
829+
if (mpys.length === 0 || isDisable) {
830+
this.clearMtply();
831+
return;
832+
}
843833
const activeExtent = hasValidExtent(smExtext) ? smExtext : mpys[0].xExtent;
844834
// rescale for zoom
845835
const { xt } = TfRescale(this);

0 commit comments

Comments
 (0)