-
Notifications
You must be signed in to change notification settings - Fork 16
Team mapml refactoring #515
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
Team mapml refactoring #515
Conversation
…m-Element into team-mapml-refactoring
…l start as "map-img" tbd.
…ustom-Element into team-mapml-refactoring
… use. Fix search and replace error on style -> map-style (link relation should stay "style").
@Malvoz @erictheise fyi this is a big change, hopefully we've covered it all. Let us know if you have concerns. We'll push the change on geogratis and experiments when this gets merged. |
Should |
I believe it would be quite disruptive, and the gain would be a bit more coherence. I tend towards "no, not enough benefit", but remain open to opinions. |
If we want to be consistent we'll also have to change I don't have a strong opinion on this, I'm mostly happy the polyfill is heading towards web platform compatibility. 😊 |
Line 965 in MapLayer.js, it's creating input elements for an extent, I believe they should be |
I think it's this, but I'll remove that code once we've got this issue closed. Should have done it before but tempus fugit :-) |
<map-title>Canada Base Map - Transportation (CBMT)</map-title> | ||
<map-meta http-equiv="Content-Type" content="text/mapml;projection=OSMTILE"></map-meta> | ||
<map-meta charset="utf-8"></map-meta> | ||
<map-link rel="license" href="https://www.nrcan.gc.ca/earth-sciences/geography/topographic-information/free-data-geogratis/licence/17285" map-title="Canada Base Map © Natural Resources Canada"></map-link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prushforth I'm looking to reflect these changes in https://github.com/Maps4HTML/MapServer-documentation/tree/mapml, I'm wondering whether it was intended to change the title
attribute on <map-link>
(s) to map-title
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming that was not intended, as it seems no other attributes use the map-
prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. We only changed the elements' prefix, I'll await further information as to whether it's necessary to change attributes. Fwiw I think the link title attribute would be appropriate if there's a conflict there that might be motivation to adopt map-title. We haven't done that yet, so I think it's best to keep with title for now. Btw I can't find the mapserver MapML code, I was looking to update it too. Lete know if you come across it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MapServer doesn't actually support MapML yet, here's the implementation ticket: MapServer/MapServer#6024.
Fixes issue #511