Skip to content

BGDIINF_SB-2716: Don't allow to edit legacy kml files#328

Merged
ltshb merged 4 commits intodevelopfrom
feat-BGDIINF_SB-2716-kml-protect
Dec 22, 2022
Merged

BGDIINF_SB-2716: Don't allow to edit legacy kml files#328
ltshb merged 4 commits intodevelopfrom
feat-BGDIINF_SB-2716-kml-protect

Conversation

@ltshb
Copy link
Contributor

@ltshb ltshb commented Dec 21, 2022 •
edited by github-actions bot
Loading

As long as we are not live on map.geo.admin.ch don't allow to edit
kml files created by the legacy viewer

This PR requires first #326

Test link

Test link with legacy kml

Test link

@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from 096ba52 to 9886d09 Compare December 21, 2022 12:34
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from 9886d09 to 95b11c8 Compare December 21, 2022 13:27
@ltshb ltshb marked this pull request as ready for review December 21, 2022 13:34
@ltshb ltshb changed the base branch from feat-BGDIINF_SB-2729-multiple-kml to develop December 21, 2022 13:46
@ltshb ltshb changed the base branch from develop to feat-BGDIINF_SB-2729-multiple-kml December 21, 2022 13:47
@ltshb ltshb requested review from boecklic and hansmannj December 21, 2022 13:50
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2729-multiple-kml branch from 5548495 to 800cc20 Compare December 21, 2022 15:24
procrastinatio
procrastinatio approved these changes Dec 21, 2022
View reviewed changes
Copy link
Contributor

@procrastinatio procrastinatio left a comment

Choose a reason for hiding this comment

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

Did only perform high level tests with
https://sys-map.dev.bgdi.ch/preview/feat-bgdiinf_sb-2716-kml-protect/index.html#/map?lang=fr&lat=47.21326&lon=7.504516&z=8.656&bgLayer=ch.swisstopo.leichte-basiskarte_world.vt&topic=ech&layers=KML|https://public.geo.admin.ch/api/kml/files/jNzproCvRcefLCiL1jFoHg|Dessin,,1

Looks OK to me, but I'm not familiar with the whole KML story (nice, editing is forbidden only for the old KML,even if many KMLs). Warning text is not translated?

@ltshb ltshb force-pushed the feat-BGDIINF_SB-2729-multiple-kml branch from 800cc20 to 7b2e701 Compare December 21, 2022 18:35
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from 95b11c8 to e24b190 Compare December 21, 2022 18:36
@ltshb ltshb requested a review from davidoesch December 21, 2022 18:37
@ltshb
Copy link
Contributor Author

ltshb commented Dec 21, 2022

Did only perform high level tests with https://sys-map.dev.bgdi.ch/preview/feat-bgdiinf_sb-2716-kml-protect/index.html#/map?lang=fr&lat=47.21326&lon=7.504516&z=8.656&bgLayer=ch.swisstopo.leichte-basiskarte_world.vt&topic=ech&layers=KML|https://public.geo.admin.ch/api/kml/files/jNzproCvRcefLCiL1jFoHg|Dessin,,1

Looks OK to me, but I'm not familiar with the whole KML story (nice, editing is forbidden only for the old KML,even if many KMLs). Warning text is not translated?

@procrastinatio I did not translate the text, because this feature is only for the MVP, once we are live with the new viewer this will be removed and we will allow to edit legacy drawing. @davidoesch do you think we should translate the kml protection warning ?

Base automatically changed from feat-BGDIINF_SB-2729-multiple-kml to develop December 21, 2022 18:50
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from e24b190 to ee2eb82 Compare December 21, 2022 18:50
@ltshb ltshb removed the request for review from boecklic December 21, 2022 18:50
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from ee2eb82 to d0c3bcf Compare December 22, 2022 06:26
pakb
pakb reviewed Dec 22, 2022
View reviewed changes
Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

I can add this disclaimer text to the translation file with in the section specific to the new viewer if we want to translate it.

jedef
jedef approved these changes Dec 22, 2022
View reviewed changes
Copy link
Contributor

@jedef jedef left a comment

Choose a reason for hiding this comment

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

It seems that I have already reivewed this by reviewing #331. For me its all good.

hansmannj
hansmannj approved these changes Dec 22, 2022
View reviewed changes
Copy link
Member

@hansmannj hansmannj left a comment

Choose a reason for hiding this comment

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

🚀 LGTM, thanks!

hansmannj
hansmannj reviewed Dec 22, 2022
View reviewed changes
ltshb added 4 commits December 22, 2022 15:07
@ltshb
BGDIINF_SB-2716: Added disabled functionality to MenuSection
1673102 
Not to disable the drawing menu we need to use the click:header event instead
of click. The click event will be fired even if disabled while the click:header
not.
@ltshb
BGDIINF_SB-2716: Don't allow to edit legacy kml files
80fa474 
As long as we are not live on `map.geo.admin.ch` don't allow to edit
kml files created by the legacy viewer
@ltshb
BGDIINF_SB-2716: Added tooltip for disabled menu drawing.
2ee4cff
@ltshb
BGDIINF_SB-2716: Added the legacy drawing warning as translated text …
336a140 
…with new translation
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2716-kml-protect branch from e654963 to 336a140 Compare December 22, 2022 14:07
@ltshb ltshb merged commit be5d11d into develop Dec 22, 2022
@ltshb ltshb deleted the feat-BGDIINF_SB-2716-kml-protect branch December 22, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hansmannj hansmannj hansmannj approved these changes

@procrastinatio procrastinatio procrastinatio approved these changes

@pakb pakb pakb approved these changes

@davidoesch davidoesch Awaiting requested review from davidoesch

+1 more reviewer

@jedef jedef jedef approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ltshb @procrastinatio @pakb @jedef @hansmannj