GPX import is not resetting markers+lines first

[carwe]

There are two buttons for GPX import - one is supposed to delete all existing markers and lines first, the other is supposed to keep them. But both work the same - they both keep the old state and merge the GPX file to it.

[carwe]

import(+reset) calls

map/src/components/sidebar_tools.ts

Line 61 in 6bb6459

this.import_gpx(files[0], true);

import+keep calls

map/src/components/sidebar_tools.ts

Line 80 in 6bb6459

this.import_gpx(files[0], false);

The bool is transported to here:

map/src/components/map_state.ts

Line 941 in 6bb6459

public from_gpx(data: string, clear: boolean): void {

and further down

map/src/components/map_state.ts

Line 1063 in 6bb6459

if (clear) {

        if (clear) {
            this.delete_all_lines();
            this.delete_all_markers();
        }

On first sight, the code looks ok-ish, and without further debugging it's not clear to me where exactly the deletion-command is lost.

[carwe]

The error is very early - both hit

map/src/components/sidebar_tools.ts

Line 80 in 6bb6459

this.import_gpx(files[0], false);

[carwe]

I don't know the first thing about JS/TS, so you absolutely need to review this, but this seems to fix the issue:

Subject: [PATCH] fix issue#42 GPX import is not resetting markers+lines first
---
Index: src/components/sidebar_tools.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/sidebar_tools.ts b/src/components/sidebar_tools.ts
--- a/src/components/sidebar_tools.ts	(revision 6bb645953f62c5c50b71d100fc1af065c746ac45)
+++ b/src/components/sidebar_tools.ts	(revision c2d2393fc8c9fca6dfa65f0a527d57f564a48d3a)
@@ -64,10 +64,10 @@
         document
             .querySelector("#btn-import-gpx-keep")!
             .addEventListener("click", (event: InputEvent): void => {
-                (document.querySelector("#inp-import-gpx") as HTMLButtonElement).click();
+                (document.querySelector("#inp-import-gpx-keep") as HTMLButtonElement).click();
                 event.preventDefault();
             });
-        (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
+        (document.querySelector("#inp-import-gpx-keep") as HTMLInputElement).onchange = (
             event: InputEvent,
         ): void => {
             if (event.target === null) {
Index: src/index.html
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/index.html b/src/index.html
--- a/src/index.html	(revision 6bb645953f62c5c50b71d100fc1af065c746ac45)
+++ b/src/index.html	(revision c2d2393fc8c9fca6dfa65f0a527d57f564a48d3a)
@@ -251,6 +251,7 @@
                         <p class="help" data-i18n="sidebar.tools.export-gpx-help">Export/import a GPX file containing map markers.</p>
                         <div class="field">
                             <div class="control">
+                                <input id="inp-import-gpx-keep" style="display: none;" type="file" name="files" />
                                 <a 
                                 id="btn-import-gpx-keep" class="button is-success is-fullwidth" data-i18n="sidebar.tools.import-gpx-keep">Import Markers (GPX); keep current state</a>
                             </div>

[carwe]

Now the second button (import+keep) doesn't trigger anything except the file chooser after using it once.

[carwe]

Reason is that the new file input was not reset. Würgaround for this is in the patch here, but, this needs to get reimplemented in a proper way. But hey, it works...

edit: removed wrong patch, complete patch in message below

[carwe]

add actual patch with fix from message above

---
Index: src/components/sidebar_tools.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/sidebar_tools.ts b/src/components/sidebar_tools.ts
--- a/src/components/sidebar_tools.ts	(revision 6bb645953f62c5c50b71d100fc1af065c746ac45)
+++ b/src/components/sidebar_tools.ts	(revision 1a90d46d9966f2be111979c5adac233c0fc12e9a)
@@ -64,10 +64,10 @@
         document
             .querySelector("#btn-import-gpx-keep")!
             .addEventListener("click", (event: InputEvent): void => {
-                (document.querySelector("#inp-import-gpx") as HTMLButtonElement).click();
+                (document.querySelector("#inp-import-gpx-keep") as HTMLButtonElement).click();
                 event.preventDefault();
             });
-        (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
+        (document.querySelector("#inp-import-gpx-keep") as HTMLInputElement).onchange = (
             event: InputEvent,
         ): void => {
             if (event.target === null) {
@@ -138,6 +138,7 @@
 
         // Reset file input
         (document.querySelector("#inp-import-gpx") as HTMLInputElement).value = "";
+        (document.querySelector("#inp-import-gpx-keep") as HTMLInputElement).value = "";
     }
 
     public export_json(): void {
Index: src/index.html
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/index.html b/src/index.html
--- a/src/index.html	(revision 6bb645953f62c5c50b71d100fc1af065c746ac45)
+++ b/src/index.html	(revision 1a90d46d9966f2be111979c5adac233c0fc12e9a)
@@ -251,6 +251,7 @@
                         <p class="help" data-i18n="sidebar.tools.export-gpx-help">Export/import a GPX file containing map markers.</p>
                         <div class="field">
                             <div class="control">
+                                <input id="inp-import-gpx-keep" style="display: none;" type="file" name="files" />
                                 <a 
                                 id="btn-import-gpx-keep" class="button is-success is-fullwidth" data-i18n="sidebar.tools.import-gpx-keep">Import Markers (GPX); keep current state</a>
                             </div>

[carwe]

The original code works like this: Both buttons simulate a click on the file-input, asking the user to select a file. The file-input yields an onchange()-event, which gets triggered once the user selected a file. The code that gets executed gets set twice (overwritten) - one for each functionality (import+keep / import+reset). And so the issue is: There is no difference which button initially got pressed - it is always the more recent (second) version of the event-handling function which gets called, and this is the one which keeps the marker/line state, regardless which button was pressed.

A second version to fix this is by specifying the onchange-handler not at load-time, but only after the button was pressed:

---
Index: src/components/sidebar_tools.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/components/sidebar_tools.ts b/src/components/sidebar_tools.ts
--- a/src/components/sidebar_tools.ts	(revision 6bb645953f62c5c50b71d100fc1af065c746ac45)
+++ b/src/components/sidebar_tools.ts	(revision a06e00150acfa2acd923ebfe60e8cfaca85071e2)
@@ -47,38 +47,38 @@
             .addEventListener("click", (event: InputEvent): void => {
                 (document.querySelector("#inp-import-gpx") as HTMLButtonElement).click();
                 event.preventDefault();
-            });
-        (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
-            event: InputEvent,
-        ): void => {
-            if (event.target === null) {
-                return;
-            }
-            const files = (event.target as HTMLInputElement).files;
-            if (files === null) {
-                return;
-            }
-            this.import_gpx(files[0], true);
-        };
+            (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
+                event: InputEvent,
+            ): void => {
+                if (event.target === null) {
+                    return;
+                }
+                const files = (event.target as HTMLInputElement).files;
+                if (files === null) {
+                    return;
+                }
+                this.import_gpx(files[0], true);
+            };
+        });
 
         document
             .querySelector("#btn-import-gpx-keep")!
             .addEventListener("click", (event: InputEvent): void => {
                 (document.querySelector("#inp-import-gpx") as HTMLButtonElement).click();
                 event.preventDefault();
-            });
-        (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
-            event: InputEvent,
-        ): void => {
-            if (event.target === null) {
-                return;
-            }
-            const files = (event.target as HTMLInputElement).files;
-            if (files === null) {
-                return;
-            }
-            this.import_gpx(files[0], false);
-        };
+                (document.querySelector("#inp-import-gpx") as HTMLInputElement).onchange = (
+                    event: InputEvent,
+                ): void => {
+                    if (event.target === null) {
+                        return;
+                    }
+                    const files = (event.target as HTMLInputElement).files;
+                    if (files === null) {
+                        return;
+                    }
+                    this.import_gpx(files[0], false);
+                };
+            });
 
         document.querySelector("#btn-export-json")!.addEventListener("click", (): void => {
             this.export_json();

(The linter cries about a non-shadowable variable "event" - it technically works, but maybe it isn't a 100% proper solution, again.)