New Custom Map UI - GeoName Search#66
Conversation
Added a new Custom map Application (GeoName Search) Which uses the GeoName API to search for Canadian Locations to map.
|
The document.querySelector("layer-").focus();The function code can be found at I'm not sure if you need it at this point but incase you were looking for it I thought I'd post it here. |
ahmadayubi
left a comment
There was a problem hiding this comment.
Everything works as expected. Some comments on consistency are:
Function names follow different conventions throughout:
GetZoomExtent()uses camel casemap_pane()is separated using underscoregeojson2mapml()uses neither of the two
For CSS class selectors I think the convention is to use - instead of _. Although it makes no functional difference.
Functionally everything looks good, and it's also a interesting demo.
|
Have only played with application so far, so have yet to review code, but I have a few suggestions: Functionality suggestions:
Code suggestions:
|
|
Are we concerned with accessibility in these custom UIs? There is no established pattern for a search results widget in the ARIA Authoring Practices, though there are some notable issues with the UI, that can be improved easily:
|
I commented at the same time as Peter, I think the answer is yes 😄 |
Does not make much difference. The misfit can probably be explained by a colleague; I will enquire. |
|
Thanks all for the extremely valuable comments. I'll make the code more consistent, work on implementing the suggestions left and also improve the UI. Accessibility is a new experience for me so thanks for all the suggestions which I completely overlooked! I'll get set up with NVDA and try to make it more friendly for screen reader users. |
* Accessibility added to application (screen-reader accessible) * Fixed UI (works on all computer screen size) * Added support for Geometry Collections * Polygon Holes are now shown * Layer Properties now viewable * + Many more minor updates TODO - make code more standard (ex. Standardizing geoJson2MapML function for broader use)
Update overflow settings for results container Co-authored-by: Robert Linder <robert.vuj.linder@outlook.com>
ahmadayubi
left a comment
There was a problem hiding this comment.
It looks good overall, I'm not sure how polished of a UI is required being that it is an experiment rather than a finished product. So I left out comments on margins and padding among the components on the page.
*updated == to === *updated variable declaration to use 'let' to be consistent
map is focused when a new layer is added
*updated the geoJson2MapMl function to now return a complete mapml layer by providing a geojson, works independently. * Application does not use templates anymore to make the geoJson2MapMl more independent
* updated naming convention * removed repetitive code * added more comments
ahmadayubi
left a comment
There was a problem hiding this comment.
Looks good overall, worked well for me.
use once instance of html parser
Malvoz
left a comment
There was a problem hiding this comment.
There is one issue which is not related to this PR but surfaces here (also Maps4HTML/MapML.js#256) due to the fact that it is the leaflet container that is the focusable component and not the map element itself; after keyboard users press enter when a location item is focused in the results list they must tab once again (how'd they know?) to be able to pan/zoom.
|
This is a great start, and just fills me with ideas in all directions. Let's merge this and improve over time. |
Added a new Custom map Application (GeoName Search) Which uses the GeoName API to search for Canadian Locations to map on the mapml-viewer