From dfb076c9ae4f57c2a015c1fa959cb44830c2c2cf Mon Sep 17 00:00:00 2001 From: hksamm Date: Sun, 26 Jul 2026 03:59:55 +0900 Subject: [PATCH] Give each Draw export button a unique element id The export button was hard-coded as id='export', with the click handler looking it up via document.getElementById('export'). Adding two Draw controls with export=True therefore emitted two elements sharing one id, which is invalid HTML and left the feature broken: every getElementById call resolved to the first button, so the second onclick assignment overwrote the first. The first button exported the second control's FeatureGroup under the second control's filename, and the second button did nothing at all. Derive the id from get_name() instead, which is what the other plugins that inject their own elements already do (see ScrollZoomToggler and FloatImage). Also drop the duplicated `top: 5px` from the button's CSS. `top: 90px` is declared later in the same block and already won, so this is dead code and the rendered position is unchanged. Adds tests/plugins/test_draw.py, which had no coverage before. Co-Authored-By: Claude Opus 5 --- folium/plugins/draw.py | 11 +++--- tests/plugins/test_draw.py | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 tests/plugins/test_draw.py diff --git a/folium/plugins/draw.py b/folium/plugins/draw.py index 7d54f1805d..2de7733bbd 100644 --- a/folium/plugins/draw.py +++ b/folium/plugins/draw.py @@ -58,9 +58,8 @@ class Draw(JSCSSMixin, MacroElement): {% macro html(this, kwargs) %} {% if this.export %} - Export + Export {% endif %} {% endmacro %} @@ -123,14 +122,14 @@ class Draw(JSCSSMixin, MacroElement): }); {% if this.export %} - document.getElementById('export').onclick = function(e) { + document.getElementById('export_{{ this.get_name() }}').onclick = function(e) { var data = drawnItems_{{ this.get_name() }}.toGeoJSON(); var convertedData = 'text/json;charset=utf-8,' + encodeURIComponent(JSON.stringify(data)); - document.getElementById('export').setAttribute( + document.getElementById('export_{{ this.get_name() }}').setAttribute( 'href', 'data:' + convertedData ); - document.getElementById('export').setAttribute( + document.getElementById('export_{{ this.get_name() }}').setAttribute( 'download', {{ this.filename|tojson }} ); } diff --git a/tests/plugins/test_draw.py b/tests/plugins/test_draw.py new file mode 100644 index 0000000000..16889b1aea --- /dev/null +++ b/tests/plugins/test_draw.py @@ -0,0 +1,70 @@ +""" +Test Draw +--------- +""" + +import re + +import folium +from folium import plugins +from folium.template import Template +from folium.utilities import normalize + + +def test_draw(): + m = folium.Map([45.0, 3.0], zoom_start=4) + draw = plugins.Draw(export=True, filename="my_data.geojson") + m.add_child(draw) + + out = normalize(m._parent.render()) + + # Verify that the export button has been created with a unique id. + tmpl = Template("Export") + assert normalize(tmpl.render(this=draw)) in out + + # Verify that the style targets that same id. + assert normalize(f"#export_{draw.get_name()} {{") in out + + # Verify that the click handler is wired to that same id. + assert ( + normalize(f"document.getElementById('export_{draw.get_name()}').onclick") in out + ) + + +def test_draw_no_export(): + m = folium.Map([45.0, 3.0], zoom_start=4) + draw = plugins.Draw() + m.add_child(draw) + + out = normalize(m._parent.render()) + + assert "Export" not in out + assert f"export_{draw.get_name()}" not in out + + +def test_two_draw_controls_get_unique_export_ids(): + """Each Draw gets its own export button, wired to its own layers. + + A hard-coded ``id='export'`` made the second button dead and pointed the + first at the wrong FeatureGroup. + """ + m = folium.Map([45.0, 3.0], zoom_start=4) + first = plugins.Draw(export=True, filename="first.geojson") + second = plugins.Draw(export=True, filename="second.geojson") + m.add_child(first) + m.add_child(second) + + out = m._parent.render() + + ids = re.findall(r"id='(export_[^']+)'", out) + assert len(ids) == 2 + assert len(set(ids)) == 2, f"export button ids are not unique: {ids}" + + # Each handler must reference its own element, layers and filename. + for draw, filename in ((first, "first.geojson"), (second, "second.geojson")): + element_id = f"export_{draw.get_name()}" + assert f"id='{element_id}'" in out + start = out.index(f"document.getElementById('{element_id}').onclick") + handler = out[start : out.index("}", start) + 1] + assert f"drawnItems_{draw.get_name()}.toGeoJSON()" in handler + assert filename in handler