Give each Draw export button a unique element id#2251
Open
Htet-Kaung-San wants to merge 1 commit into
Open
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
Drawexport button is hard-coded asid='export', and its click handler looks it up withdocument.getElementById('export'). Adding twoDrawcontrols withexport=Trueemits two elements sharing one id — invalid HTML, and the feature breaks:On
mainthat renders two<a id='export'>elements. EverygetElementById('export')call resolves to the first one, so the secondonclickassignment overwrites the first. The result is that the first button exports the second control'sFeatureGroupunder 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 —ScrollZoomTogglerandFloatImageboth useid="{{this.get_name()}}".I also dropped the duplicated
top: 5pxfrom the button's CSS.top: 90pxis 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.pyis new — the Draw plugin had no test coverage. It covers the unique id, theexport=Falsecase, and two controls each staying wired to their ownFeatureGroupand filename. The two id-related tests fail onmainand pass here.Full suite, excluding
tests/seleniumandtests/snapshots(missingpixelmatchlocally): 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 threetest_repr.pypng tests — so none are from this change.ruffandcodespellare clean.blackwants to rewrap theTemplate(...)call indraw.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.