From 35189cc9fbdb8c2582abefebe757c7ec64fc610a Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 5 Jun 2024 18:37:02 +0200 Subject: [PATCH] wip(importer): use only one submit button and be smarter to guess action --- umap/static/umap/base.css | 4 + umap/static/umap/js/modules/importer.js | 68 +++++++++-------- umap/tests/integration/test_import.py | 97 ++++++++++++++++--------- 3 files changed, 104 insertions(+), 65 deletions(-) diff --git a/umap/static/umap/base.css b/umap/static/umap/base.css index 8327057f..a79943b2 100644 --- a/umap/static/umap/base.css +++ b/umap/static/umap/base.css @@ -300,6 +300,10 @@ input + .help-text { .formbox select { width: calc(100% - 14px); } +fieldset.formbox { + border: none; + border-top: 1px solid var(--color-lightGray); +} label { display: block; font-size: 12px; diff --git a/umap/static/umap/js/modules/importer.js b/umap/static/umap/js/modules/importer.js index 2aefc9eb..479dce3b 100644 --- a/umap/static/umap/js/modules/importer.js +++ b/umap/static/umap/js/modules/importer.js @@ -6,33 +6,41 @@ import { SCHEMA } from './schema.js' const TEMPLATE = `

${translate('Add data')}

- -
+
+ ${translate('Choose data')} -
-
-
${translate('Import helpers:')}
-
+
+
${translate('Import helpers:')}
+
+
-
-
- - - - + + ` export default class Importer { @@ -86,6 +94,10 @@ export default class Importer { return Boolean(this.qs('[name=clear]').checked) } + get action() { + return this.qs('[name=action]:checked').value + } + get layer() { const layerId = this.qs('[name=layer-id]').value return this.map.datalayers[layerId] || this.map.createDataLayer() @@ -113,9 +125,7 @@ export default class Importer { textContent: type, }) } - DomEvent.on(this.qs('[name=copy]'), 'click', this.copy, this) - DomEvent.on(this.qs('[name=link]'), 'click', this.link, this) - DomEvent.on(this.qs('[name=full]'), 'click', this.full, this) + DomEvent.on(this.qs('[name=submit]'), 'click', this.submit, this) DomEvent.on(this.qs('[type=file]'), 'change', this.onFileChange, this) for (const element of this.container.querySelectorAll('[onchange]')) { DomEvent.on(element, 'change', this.onChange, this) @@ -123,13 +133,8 @@ export default class Importer { } onChange() { - this.qs('[name=link]').toggleAttribute( - 'hidden', - !this.url || this.format === 'umap' - ) - this.qs('[name=full]').toggleAttribute('hidden', this.format !== 'umap') - this.qs('[name=copy]').toggleAttribute('hidden', this.format === 'umap') this.qs('.destination').toggleAttribute('hidden', this.format === 'umap') + this.qs('.import-mode').toggleAttribute('hidden', this.format === 'umap' || !this.url) } onFileChange(e) { @@ -181,8 +186,13 @@ export default class Importer { this.fileInput.showPicker() } + submit() { + if (this.format === 'umap') this.full() + else if (!this.url) this.copy() + else if (this.action) this[this.action]() + } + full() { - if (this.format !== 'umap') return this.map.once('postsync', this.map._setDefaultCenter) try { if (this.files.length) { diff --git a/umap/tests/integration/test_import.py b/umap/tests/integration/test_import.py index 6f8fa451..a124b50c 100644 --- a/umap/tests/integration/test_import.py +++ b/umap/tests/integration/test_import.py @@ -17,20 +17,16 @@ def test_layers_list_is_updated(live_server, tilelayer, page): modifier = "Cmd" if platform.system() == "Darwin" else "Ctrl" page.get_by_role("link", name=f"Import data ({modifier}+I)").click() # Should work - page.get_by_label("Choose the layer to import").select_option( - label="Import in a new layer" - ) + page.locator("[name=layer-id]").select_option(label="Import in a new layer") page.get_by_role("link", name="Manage layers").click() page.get_by_role("button", name="Add a layer").click() page.locator('input[name="name"]').click() page.locator('input[name="name"]').fill("foobar") page.get_by_role("link", name=f"Import data ({modifier}+I)").click() # Should still work - page.get_by_label("Choose the layer to import").select_option( - label="Import in a new layer" - ) + page.locator("[name=layer-id]").select_option(label="Import in a new layer") # Now layer should be visible in the options - page.get_by_label("Choose the layer to import").select_option(label="foobar") + page.locator("[name=layer-id]").select_option(label="foobar") expect( page.get_by_role("button", name="Import full map data", exact=True) ).to_be_hidden() @@ -48,12 +44,13 @@ def test_umap_import_from_file(live_server, tilelayer, page): file_chooser = fc_info.value path = Path(__file__).parent.parent / "fixtures/display_on_load.umap" file_chooser.set_files(path) - button = page.get_by_role("button", name="Import full map data", exact=True) expect( page.get_by_role("button", name="Copy into the layer", exact=True) ).to_be_hidden() - expect(button).to_be_visible() - button.click() + expect( + page.get_by_role("button", name="Link to the layer as remote data", exact=True) + ).to_be_hidden() + page.get_by_role("button", name="Add data", exact=True).click() assert file_input.input_value() # Close the import panel page.keyboard.press("Escape") @@ -80,7 +77,7 @@ def test_umap_import_from_textarea(live_server, tilelayer, page, settings): path = Path(__file__).parent.parent / "fixtures/test_upload_data.umap" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("umap") - page.get_by_role("button", name="Import full map data", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() layers = page.locator(".umap-browser .datalayer") expect(layers).to_have_count(2) expect(page.locator(".umap-main-edit-toolbox .map-name")).to_have_text( @@ -115,9 +112,7 @@ def test_import_geojson_from_textarea(tilelayer, live_server, page): path = Path(__file__).parent.parent / "fixtures/test_upload_data.json" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("geojson") - button = page.get_by_role("button", name="Copy into the layer", exact=True) - expect(button).to_be_visible() - button.click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(2) @@ -140,9 +135,7 @@ def test_import_kml_from_textarea(tilelayer, live_server, page): path = Path(__file__).parent.parent / "fixtures/test_upload_data.kml" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("kml") - button = page.get_by_role("button", name="Copy into the layer", exact=True) - expect(button).to_be_visible() - button.click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(1) @@ -165,9 +158,7 @@ def test_import_gpx_from_textarea(tilelayer, live_server, page): path = Path(__file__).parent.parent / "fixtures/test_upload_data.gpx" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("gpx") - button = page.get_by_role("button", name="Copy into the layer", exact=True) - expect(button).to_be_visible() - button.click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(1) @@ -188,7 +179,7 @@ def test_import_osm_from_textarea(tilelayer, live_server, page): path = Path(__file__).parent.parent / "fixtures/test_upload_data_osm.json" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("osm") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(2) @@ -208,7 +199,7 @@ def test_import_csv_from_textarea(tilelayer, live_server, page): path = Path(__file__).parent.parent / "fixtures/test_upload_data.csv" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("csv") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(2) @@ -227,7 +218,7 @@ def test_can_import_in_existing_datalayer(live_server, datalayer, page, openmap) path = Path(__file__).parent.parent / "fixtures/test_upload_data.csv" textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("csv") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # No layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(3) @@ -247,7 +238,7 @@ def test_can_replace_datalayer_data(live_server, datalayer, page, openmap): textarea.fill(path.read_text()) page.locator('select[name="format"]').select_option("csv") page.get_by_label("Replace layer content").check() - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # No layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(2) @@ -265,11 +256,9 @@ def test_can_import_in_new_datalayer(live_server, datalayer, page, openmap): textarea = page.locator(".umap-upload textarea") path = Path(__file__).parent.parent / "fixtures/test_upload_data.csv" textarea.fill(path.read_text()) - page.locator('select[name="format"]').select_option("csv") - page.get_by_label("Choose the layer to import").select_option( - label="Import in a new layer" - ) - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.locator("select[name=format]").select_option("csv") + page.locator("[name=layer-id]").select_option(label="Import in a new layer") + page.get_by_role("button", name="Add data", exact=True).click() # A new layer has been created expect(layers).to_have_count(2) expect(markers).to_have_count(3) @@ -311,7 +300,7 @@ def test_should_remove_dot_in_property_names(live_server, page, settings, tilela textarea = page.locator(".umap-upload textarea") textarea.fill(json.dumps(data)) page.locator('select[name="format"]').select_option("geojson") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() with page.expect_response(re.compile(r".*/datalayer/create/.*")): page.get_by_role("button", name="Save").click() datalayer = DataLayer.objects.last() @@ -372,7 +361,7 @@ def test_import_geometry_collection(live_server, page, tilelayer): textarea = page.locator(".umap-upload textarea") textarea.fill(json.dumps(data)) page.locator('select[name="format"]').select_option("geojson") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(markers).to_have_count(1) @@ -406,7 +395,7 @@ def test_import_multipolygon(live_server, page, tilelayer): textarea = page.locator(".umap-upload textarea") textarea.fill(json.dumps(data)) page.locator('select[name="format"]').select_option("geojson") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(paths).to_have_count(1) @@ -438,7 +427,7 @@ def test_import_multipolyline(live_server, page, tilelayer): textarea = page.locator(".umap-upload textarea") textarea.fill(json.dumps(data)) page.locator('select[name="format"]').select_option("geojson") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # A layer has been created expect(layers).to_have_count(1) expect(paths).to_have_count(1) @@ -453,7 +442,7 @@ def test_import_csv_without_valid_latlon_headers(tilelayer, live_server, page): textarea = page.locator(".umap-upload textarea") textarea.fill("a,b,c\n12.23,48.34,mypoint\n12.23,48.34,mypoint2") page.locator('select[name="format"]').select_option("csv") - page.get_by_role("button", name="Copy into the layer", exact=True).click() + page.get_by_role("button", name="Add data", exact=True).click() # FIXME do not create a layer expect(layers).to_have_count(1) expect(markers).to_have_count(0) @@ -485,8 +474,9 @@ def test_create_remote_data(page, live_server, tilelayer): page.get_by_role("link", name="Import data (Ctrl+I)").click() page.get_by_placeholder("Provide an URL here").click() page.get_by_placeholder("Provide an URL here").fill("https://remote.org/data.json") - page.get_by_label("Choose the data format").select_option("geojson") - page.get_by_role("button", name="Link to the layer as remote").click() + page.locator("[name=format]").select_option("geojson") + page.get_by_role("radio", name="Link to the layer as remote data").click() + page.get_by_role("button", name="Add data", exact=True).click() expect(page.locator(".leaflet-marker-icon")).to_be_visible() page.get_by_role("link", name="Manage layers").click() page.get_by_role("button", name="Edit", exact=True).click() @@ -494,3 +484,38 @@ def test_create_remote_data(page, live_server, tilelayer): expect(page.locator('input[name="url"]')).to_have_value( "https://remote.org/data.json" ) + + +def test_import_geojson_from_url(page, live_server, tilelayer): + def handle(route): + route.fulfill( + json={ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Point", + "coordinates": [4.3375, 51.2707], + }, + } + ], + } + ) + + # Intercept the route to the proxy + page.route("https://remote.org/data.json", handle) + page.goto(f"{live_server.url}/map/new/") + expect(page.locator(".leaflet-marker-icon")).to_be_hidden() + page.get_by_role("link", name="Import data (Ctrl+I)").click() + page.get_by_placeholder("Provide an URL here").click() + page.get_by_placeholder("Provide an URL here").fill("https://remote.org/data.json") + page.locator("[name=format]").select_option("geojson") + page.get_by_role("radio", name="Copy into the layer").click() + page.get_by_role("button", name="Add data", exact=True).click() + expect(page.locator(".leaflet-marker-icon")).to_be_visible() + page.get_by_role("link", name="Manage layers").click() + page.get_by_role("button", name="Edit", exact=True).click() + page.locator("summary").filter(has_text="Remote data").click() + expect(page.locator('input[name="url"]')).to_have_value("")