Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PB-856 : add a style selector for KML in layer settings #1047

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

pakb
Copy link
Contributor

@pakb pakb commented Aug 30, 2024

this enables users to opt-out of the default style (Google Earth like) we apply by default to external KML, and let them go back to the GeoAdmin (all red) style that was previously (on mf-geoadmin3) applied across the board.

adding this choice to the layer URL extra attributes

Test link

Copy link

cypress bot commented Aug 30, 2024

web-mapviewer    Run #3872

Run Properties:  status check passed Passed #3872  •  git commit f26ca6b20a: PB-856 : add a style selector for KML in layer settings
Project web-mapviewer
Branch Review feat-PB-856-add-kml-style-selector
Run status status check passed Passed #3872
Run duration 04m 46s
Commit git commit f26ca6b20a: PB-856 : add a style selector for KML in layer settings
Committer Pascal Barth
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 212
View all changes introduced in this branch ↗︎

@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch 4 times, most recently from 0787d65 to 185ba67 Compare September 11, 2024 14:50
@pakb pakb requested review from ltshb and ltkum September 11, 2024 15:22
@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from 185ba67 to 4b0ac63 Compare September 13, 2024 09:21
@pakb pakb requested a review from ltshb September 13, 2024 12:37
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When opening the drawing menu and clicking on add icon, the icon is missing and you have the following type error
image

@ltshb
Copy link
Contributor

ltshb commented Sep 16, 2024

@pakb @hansmannj With styling and icons we might have a general issue with the introduction of babs icons, this PR (kml style selector) and PB-64 self-contained KML. I did not yet check and tested the babs icon for legacy icons, but did you think about this ? What happen if you open and edit a drawing with legacy babs icon ?
Currently (at least before babs icon implementation) the icon selector was a mapping between icon url from the drawing and our icons service which only work if the url was found in our service, now with legacy unlisted icons what do we do ?
The same question will also apply for self-contain kml where the icon might not be found in our service-icons if we deprecated them, what do we do in this case ?
The response of these questions might impact the style selector, we should have a discussion about this tomorrow.

@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from 4b0ac63 to 33725a6 Compare October 2, 2024 08:23
@pakb pakb requested a review from ismailsunni October 2, 2024 08:25
@ismailsunni
Copy link
Contributor

I tried to load Rossier.kml from the JIRA ticket, but it doesn't allow me anymore. It works in production
image

@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from 33725a6 to 4cb37d2 Compare November 1, 2024 16:30
@hansmannj
Copy link
Member

@pakb @ismailsunni @ltshb : What's the status here?
I think the issues with the BABS icons should be solved. We keep on providing the old icons but unlist them in the viewer. What else needs to be clarified for this PR to be merged?

@ismailsunni
Copy link
Contributor

using this PR test link:

  • I no longer able to load KML file using the import file tool, but I can still load it by using drag and drop

@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from 4cb37d2 to 6352abe Compare November 5, 2024 12:22
@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from 6352abe to f85800a Compare November 20, 2024 06:30
this enables users to opt-out of the default style (Google Earth like) we apply by default to external KML, and let them go back to the GeoAdmin (all red) style that was previously (on mf-geoadmin3) applied across the board.

adding this choice to the layer URL extra attributes
@pakb pakb force-pushed the feat-PB-856-add-kml-style-selector branch from f85800a to f26ca6b Compare November 20, 2024 07:39
Copy link
Contributor

@ismailsunni ismailsunni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the issue I found aboe is fixed also

@pakb pakb merged commit b81950d into develop Nov 20, 2024
6 checks passed
@pakb pakb deleted the feat-PB-856-add-kml-style-selector branch November 20, 2024 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants