Skip to content

updating in place? #1022

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

Open
Fil opened this issue Aug 4, 2022 · 4 comments
Open

updating in place? #1022

Fil opened this issue Aug 4, 2022 · 4 comments
Labels
question Further information is needed

Comments

@Fil
Copy link
Contributor

Fil commented Aug 4, 2022

If we want to update the marks in place in the DOM rather than throw them away and create a new one on each tick (in the context of the time animations, and later interactions), we can pass the previous node as part of the context (as in the patch below), and use join rather than append.

(And then, why not go further and always pass a g element, on any call to render — that is, independently of whether it's a previously rendered node or a new one just created with create("svg:g", context)? The mark would still be free to ignore it and return something else, of course.)

diff --git a/src/marks/dot.js b/src/marks/dot.js
index e4140b55..3442267f 100644
--- a/src/marks/dot.js
+++ b/src/marks/dot.js
@@ -1,4 +1,4 @@
-import {path, symbolCircle} from "d3";
+import {path, select, symbolCircle} from "d3";
 import {create} from "../context.js";
 import {positive} from "../defined.js";
 import {identity, maybeFrameAnchor, maybeNumberChannel, maybeTuple} from "../options.js";
@@ -55,13 +55,12 @@ export class Dot extends Mark {
     const {x: X, y: Y, r: R, rotate: A, symbol: S} = channels;
     const [cx, cy] = applyFrameAnchor(this, dimensions);
     const circle = this.symbol === symbolCircle;
-    return create("svg:g", context)
+    return (context.node ? select(context.node) : create("svg:g", context)
         .call(applyIndirectStyles, this, scales, dimensions)
-        .call(applyTransform, this, scales)
-        .call(g => g.selectAll()
+        .call(applyTransform, this, scales))
+        .call(g => g.selectAll(circle ? "circle" : "path")
           .data(index, i => i)
-          .enter()
-          .append(circle ? "circle" : "path")
+          .join(circle ? "circle" : "path")
             .call(applyDirectStyles, this)
             .call(circle
               ? selection => {
diff --git a/src/plot.js b/src/plot.js
index 51864c40..6a9ec847 100644
--- a/src/plot.js
+++ b/src/plot.js
@@ -376,7 +376,7 @@ export function plot(options = {}) {
           }
           const ifacet = [...facet.filter(i => T[i] < time1), ...(currentTime < time1) ? Ii : [], ...facet.filter(i => T[i] >= time1)];
           const index = mark.timeFilter(ifacet, T, currentTime);
-          timeNode = mark.render(index, scales, interp, dimensions, context);
+          timeNode = mark.render(index, scales, interp, dimensions, {...context, node: timeMark.node});
         } else {
           // here typically t = 0;
           const index = mark.timeFilter(facet, T, currentTime);
@Fil Fil added the question Further information is needed label Aug 4, 2022
@Fil Fil mentioned this issue Aug 4, 2022
@Fil
Copy link
Contributor Author

Fil commented Aug 10, 2022

I think we need this not only for performance, but to allow users to click on the chart—currently if you click on a mark while it's updating, the event does not bubble up and reach the chart (probably because the svg element you clicked has been withdrawn from the DOM and replaced with a new one).

@Fil
Copy link
Contributor Author

Fil commented May 11, 2023

(To reconsider in light of the new render transform.)

@Fil
Copy link
Contributor Author

Fil commented Jun 5, 2023

@Fil
Copy link
Contributor Author

Fil commented Jul 3, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is needed
Projects
None yet
Development

No branches or pull requests

1 participant