diff --git a/drawdata/static/bar_widget.js b/drawdata/static/bar_widget.js index 3b47cf5..587ea3a 100644 --- a/drawdata/static/bar_widget.js +++ b/drawdata/static/bar_widget.js @@ -9554,6 +9554,19 @@ function render({ model, el }) { let minY = model.get("y_min"); let maxY = model.get("y_max"); let data; + let syncingDataOut = false; + function seedFromModel() { + Object.keys(collections).forEach((key) => { + collections[key].data = new Array(getBins()).fill(model.get("y_min")); + }); + (model.get("data") || []).forEach((d) => { + const collection = collections[d.collection]; + if (collection && d.bin >= 0 && d.bin < collection.data.length) { + collection.data[d.bin] = Math.max(minY, Math.min(maxY, d.value)); + } + }); + } + seedFromModel(); updateDataOut(); getBins(); let container = document.createElement("div"); @@ -9654,10 +9667,35 @@ function render({ model, el }) { data.push({ collection: key, value, bin: j, color: collections[key].color }); }); }); + syncingDataOut = true; model.set("data", data); model.save_changes(); + syncingDataOut = false; } ; + model.on("change:data", function() { + if (syncingDataOut) + return; + seedFromModel(); + updateChart(); + }); + model.on("change:n_bins", function() { + Object.keys(collections).forEach((key) => { + const old = collections[key].data; + collections[key].data = d3.range(getBins()).map( + (i) => i < old.length ? old[i] : model.get("y_min") + ); + }); + updateChart(); + }); + model.on("change:y_min", function() { + minY = model.get("y_min"); + updateChart(); + }); + model.on("change:y_max", function() { + maxY = model.get("y_max"); + updateChart(); + }); const chartArea = svg.append("rect").attr("width", width).attr("height", height).attr("fill", "none").attr("pointer-events", "all"); function updateValue(event) { const [xPos, yPos] = d3.pointer(event); @@ -9683,9 +9721,9 @@ function render({ model, el }) { chartArea.on("mouseleave", () => { isDrawing = false; }); - document.querySelectorAll("button.control").forEach((button) => { + container.querySelectorAll("button.control").forEach((button) => { button.addEventListener("click", () => { - document.querySelectorAll("button.control").forEach((b) => b.classList.remove("active")); + container.querySelectorAll("button.control").forEach((b) => b.classList.remove("active")); button.classList.add("active"); activeCollection = button.querySelector("span").textContent; }); @@ -9697,7 +9735,7 @@ function render({ model, el }) { updateChart(); }); if (Object.keys(collections).length > 1) { - document.querySelector("button.control").click(); + container.querySelector("button.control").click(); } updateChart(); } diff --git a/js/bar_widget.js b/js/bar_widget.js index 43d4008..3d1b2f7 100644 --- a/js/bar_widget.js +++ b/js/bar_widget.js @@ -24,7 +24,28 @@ function render({ model, el }) { let isDrawing = false; let minY = model.get("y_min"); let maxY = model.get("y_max"); - let data; + let data; + // True while updateDataOut() is syncing data to the model, so the + // change:data handler below can tell its own echo apart from a real + // Python-side update (model.set() fires change synchronously) + let syncingDataOut = false; + + // (Re)fill each collection's bins from the model's data, so data passed + // to the constructor (or set later from Python) is reflected in the + // chart. Bins the data doesn't cover fall back to y_min, and values are + // clamped to the [y_min, y_max] range like drawn values are. + function seedFromModel() { + Object.keys(collections).forEach(key => { + collections[key].data = new Array(getBins()).fill(model.get("y_min")); + }); + (model.get("data") || []).forEach(d => { + const collection = collections[d.collection]; + if (collection && d.bin >= 0 && d.bin < collection.data.length) { + collection.data[d.bin] = Math.max(minY, Math.min(maxY, d.value)); + } + }); + } + seedFromModel(); updateDataOut(); // Create SVG @@ -207,10 +228,39 @@ function render({ model, el }) { data.push({collection: key, value: value, bin: j, color: collections[key].color}); }) }); + syncingDataOut = true; model.set("data", data); model.save_changes(); + syncingDataOut = false; }; + // React to Python-side updates (ScatterWidget already does this via its + // own change: handlers) + model.on("change:data", function() { + if (syncingDataOut) return; + seedFromModel(); + updateChart(); + }); + model.on("change:n_bins", function() { + // Re-fit each collection to the new bin count, keeping the bins + // that still exist + Object.keys(collections).forEach(key => { + const old = collections[key].data; + collections[key].data = d3.range(getBins()).map(i => + i < old.length ? old[i] : model.get("y_min") + ); + }); + updateChart(); + }); + model.on("change:y_min", function() { + minY = model.get("y_min"); + updateChart(); + }); + model.on("change:y_max", function() { + maxY = model.get("y_max"); + updateChart(); + }); + // Handle mouse events const chartArea = svg.append("rect") .attr("width", width) @@ -248,10 +298,13 @@ function render({ model, el }) { isDrawing = false; }); - // Handle collection selection - document.querySelectorAll('button.control').forEach(button => { + // Handle collection selection. Scope the queries to this widget's own + // container: document-level queries throw when the view isn't attached + // to the document yet (some hosts render before attaching), and make + // multiple BarWidgets on a page switch each other's active collection. + container.querySelectorAll('button.control').forEach(button => { button.addEventListener('click', () => { - document.querySelectorAll('button.control').forEach(b => b.classList.remove('active')); + container.querySelectorAll('button.control').forEach(b => b.classList.remove('active')); button.classList.add('active'); activeCollection = button.querySelector('span').textContent; }); @@ -266,7 +319,7 @@ function render({ model, el }) { }); if (Object.keys(collections).length > 1) { - document.querySelector('button.control').click(); + container.querySelector('button.control').click(); } // Initialize chart diff --git a/tests/test_bar_widget_python_sync.py b/tests/test_bar_widget_python_sync.py new file mode 100644 index 0000000..03c203a --- /dev/null +++ b/tests/test_bar_widget_python_sync.py @@ -0,0 +1,38 @@ +from pathlib import Path + + +BUNDLE = Path("drawdata/static/bar_widget.js").read_text(encoding="utf-8") + + +def test_bar_widget_seeds_from_model_data(): + # render() must read the model's existing data into the collections + # (so constructor-provided data survives the first render) + assert "seedFromModel" in BUNDLE + assert 'model.get("data")' in BUNDLE + + +def test_bar_widget_listens_for_python_side_updates(): + # ScatterWidget already has change: handlers; BarWidget needs them too, + # otherwise Python-side updates only apply on a re-render + assert 'model.on("change:data"' in BUNDLE + assert 'model.on("change:n_bins"' in BUNDLE + assert 'model.on("change:y_min"' in BUNDLE + assert 'model.on("change:y_max"' in BUNDLE + + +def test_bar_widget_change_data_handler_ignores_own_echo(): + # updateDataOut() calls model.set("data", ...), which fires change:data + # synchronously; without a guard the handler would loop + assert "syncingDataOut" in BUNDLE + + +def test_bar_widget_scopes_button_queries_to_its_container(): + # document.querySelector('button.control') throws when the view isn't + # attached to the document yet, and cross-contaminates multiple + # BarWidgets on a page; queries must go through the widget's container + assert 'document.querySelector(\'button.control\')' not in BUNDLE + assert 'document.querySelectorAll(\'button.control\')' not in BUNDLE + assert 'document.querySelector("button.control")' not in BUNDLE + assert 'document.querySelectorAll("button.control")' not in BUNDLE + assert 'container.querySelector("button.control")' in BUNDLE + assert 'container.querySelectorAll("button.control")' in BUNDLE