Skip to content

Give each Draw export button a unique element id#2251

Open
Htet-Kaung-San wants to merge 1 commit into
python-visualization:mainfrom
Htet-Kaung-San:fix/draw-export-unique-id
Open

Give each Draw export button a unique element id#2251
Htet-Kaung-San wants to merge 1 commit into
python-visualization:mainfrom
Htet-Kaung-San:fix/draw-export-unique-id

Conversation

@Htet-Kaung-San

Copy link
Copy Markdown

The Draw export button is hard-coded as id='export', and its click handler looks it up with document.getElementById('export'). Adding two Draw controls with export=True emits two elements sharing one id — invalid HTML, and the feature breaks:

import folium
from folium.plugins import Draw

m = folium.Map()
Draw(export=True, filename="a.geojson").add_to(m)
Draw(export=True, filename="b.geojson").add_to(m)
m.save("out.html")

On main that renders two <a id='export'> elements. Every getElementById('export') call resolves to the first one, so the second onclick assignment overwrites the first. The result is that the first button exports the second control's FeatureGroup under the second control's filename, and the second button does nothing at all.

This derives the id from get_name() instead, which is what the other plugins injecting their own elements already do — ScrollZoomToggler and FloatImage both use id="{{this.get_name()}}".

I also dropped the duplicated top: 5px from the button's CSS. top: 90px is declared later in the same block and already won, so it was dead code — the rendered position is unchanged.

On the id change

The id is not documented as public API, but anyone currently styling the button with a #export { ... } rule would need to switch to targeting the element another way. Happy to keep a stable class alongside the unique id if you'd rather not break that.

I found this while reading the plugin for #1806; this PR doesn't address that request.

Testing

tests/plugins/test_draw.py is new — the Draw plugin had no test coverage. It covers the unique id, the export=False case, and two controls each staying wired to their own FeatureGroup and filename. The two id-related tests fail on main and pass here.

Full suite, excluding tests/selenium and tests/snapshots (missing pixelmatch locally): 258 passed, 6 failed. The same 6 fail identically on an unmodified checkout — test_timedynamic_geo_json, test_dead_map_id_rule_absent, test_icon_invalid_marker_colors, and three test_repr.py png tests — so none are from this change.

ruff and codespell are clean. black wants to rewrap the Template(...) call in draw.py, but it asks for exactly that same rewrap on an unmodified checkout; I get black 25.11.0 locally since 26.5.1 doesn't install on Python 3.9, so I left the formatting alone rather than add unrelated churn.

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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant