Announce zoom and move and implementing map boundaries #533

ben-lu-uw · 2021-10-14T18:31:52Z

This implements meaningful pan and zoom from issue #396 and "bounce back" when max/min pan has been reached.
As of the making of this draft this functionality has only been implemented for mapml-viewer + templatedtilelayers and only been tested on tms.html. Currently looking for feedback.
Known issues:

When the map is dragged out of bounds the return behavior is inconsistent
Max/min zoom announcement has only been implemented with +/- keys
Have not considered the behavior when multiple layers are present

ahmadayubi · 2021-10-14T18:54:12Z

You may want to consider creating a new handler that extends L.Handler similar to ContextMenu.js. Then in that handler you can listen for zoom and moveend events in one place without having to modify all the different moveend handlers in the layers.

You would also be able to conditionally add the new handler. So you'd only need to check if M.options.announceMovement is true or not once rather than on every moveend.

For contextmenu, it looks like this,
https://github.com/Maps4HTML/Web-Map-Custom-Element/blob/14375f26c641a03c74cb798856d63a9c232caa27/src/mapml-viewer.js#L196-L201

For announceMovement, you could do the same and instead of true pass in M.options.announceMovement.

This is a good resource: https://leafletjs.com/examples/extending/extending-3-controls.html

prushforth · 2021-10-14T19:40:17Z

src/mapml-viewer.js

@@ -5,6 +5,32 @@ import './mapml.js';       // refactored URI usage, replaced with URL standard
 import './Leaflet.fullscreen.js';
 import { MapLayer } from './layer.js';

+class ExtendedMap extends L.Map {


I am not sure it's a good idea to include one class definition in the definition file for another. Maybe it's own class file, if indeed a class is necessary.

We have so far not had to extend L.Map, so would try to stay away from that. If that's impossible, I don't think that Leaflet yet (properly) supports ES6 modules, it has its own extension mechanism, see here: https://leafletjs.com/examples/extending/extending-1-classes.html and here https://leafletjs.com/examples/extending/extending-3-controls.html

prushforth · 2021-10-14T19:52:49Z

src/mapml-viewer.js

@@ -100,7 +126,8 @@ export class MapViewer extends HTMLElement {
  constructor() {
    // Always call super first in constructor
    super();
-
+    this.insertAdjacentHTML("beforebegin", "<output role='status' aria-live='polite' aria-atomic='true' id='screenReaderOutput' style='clip: rect(0 0 0 0);clip-path: inset(50%);height: 1px;overflow: hidden;position: absolute;white-space: nowrap;width: 1px;'></output>");


I apologize, I think I mentioned an "attached" method, which no longer is part of custom elements; I should have said "connectedCallback" might be a good location for this. However, I thought I remembered from somewhere that you shouldn't do DOM manipulation in the constructor, but in reading our own code, that doesn't make sense, so maybe where you've got it is correct TBD.

Anyhow, for reference, some best practices: https://developers.google.com/web/fundamentals/web-components/best-practices#create-your-shadow-root-in-the-constructor.

src/mapml-viewer.js

ahmadayubi · 2021-10-25T18:13:23Z

src/mapml/handlers/AnnounceMovement.js

+    addHooks: function () {
+        this._map.on({
+            moveend: this.announceBounds,
+            zoomend: this.totalBounds,


If the bounds are in terms of pcrs you only need to calculate it once at the beginning and whenever a layer is added or removed from the map.

In the above you can see that on different zoom levels the value this function sets doesn't change.

ahmadayubi · 2021-10-25T18:16:01Z

src/mapml/handlers/AnnounceMovement.js

+            }
+        });
+
+        this.totalLayerBounds = bounds;


The calculated bounds seems to be static, if a layer is deselected the value doesn't reflect that. If a layer is deselected I'm not sure if it should be considered, unless that was something that was decided on.

ahmadayubi · 2021-10-25T18:22:21Z

src/mapml/handlers/AnnounceMovement.js

+        let visible = mapZoom <= this._layersMaxZoom && mapZoom >= this._layersMinZoom &&
+            this.totalLayerBounds.overlaps(mapBounds);
+
+        if(!this.totalLayerBounds){


!this.totalLayerBounds is checked after the variable is already used, the mapZoom <= this._layersMaxZoom && mapZoom >= this._layersMinZoom && this.totalLayerBounds.overlaps(mapBounds); could be conditionally called, something like:

let visible = true; if(this.totalLayerBounds){ visible = mapZoom <= this._layersMaxZoom && mapZoom >= this._layersMinZoom && this.totalLayerBounds.overlaps(mapBounds); }

ahmadayubi · 2021-10-25T18:31:51Z

src/mapml/handlers/AnnounceMovement.js

+        let standard = "zoom level " + mapZoom + " column " + column + " row " + row;
+
+        if(!visible){
+            mapEl._addToHistory();


Manually adding to history may cause other issues, one I noticed was after being out of bounds you cannot go back (the index also has other odd behaviors):

2021-10-25.14-29-28.mp4

ben-lu-uw · 2021-10-26T03:46:53Z

Hopefully resolved the indexing issues but now when reaching the top bounds, the map bounces back to an unexpected location on certain occasions.
Edit: seems like it also happens on the main branch as well:

2021-10-26.11-26-08.mov

Moved to issue #542

prushforth · 2021-11-04T19:56:35Z

@Malvoz why don't links announce where they go? For map links, I think it would be good to know whether the link will navigate the page or the map, for example. Do web links not have that notion? For visual users, we see the destination of the link in the bottom left of the screen/map, so there's some sense that we're not going to hell by clicking on the link. Do non-visual users not get that same assurance somehow?

Malvoz · 2021-11-04T20:29:00Z

@prushforth To my knowledge links are always just announced "link" along with their accessible name/desc, even when they're target="_blank" for example. So no, non-visual users do not get that same assurance, I believe. Which is bad for screen reader users, but good for us since we're using fake links and would otherwise have had to replicate and translate that language.

Create util function for zoom/movement screen reader support Fix setTimeout Focus on the map Focus on the map Fix focus on the map Add announceMoveAndZoom functionality to all layer types Use aria-label for announce zoom and move Use output element for announcing zoom and move Add/Fix zoom and pan bounds Add dragging bounds Create new handler to listen for move events Add bounds Implement combined bounds to handle multiple layers bound check [work in progress] Implement combined bounds to handle multiple layers bound check [work in progress] Fix total bounds and bounds check Add total bounds rectangle to debug layer Change output element and total layer bounds rectangle position in dom to satisfy tests Disable bounds check when no bounds are present Refactor output element Set initial bounds to center of first bounds instead of [0,0] Clean up code Refactor output element class name Resolve indexing issues [work in progress] Make deselected layers not considered for the total bounds Use layeradd/layerremove instead of checkdisable for timing reasons Announce location on focus Fix max/min zoom announcements Fix dragged out of bounds condition Fix dragged out of bounds condition Merge in history fix Fix double moveend call issue Remove console log Add announceMovement test

prushforth

I take it you have to reload the map for the extension setting to take effect on announcing/not announcing map location. In general the experience is what I expect, more or less; a few extra things that I don't recognized are announced. In this example page, when you tab to the second map, initially the location isn't announced. rows and columns are projection specific, we may need to figure out improvements incrementally. For example, as you remove layers from the map, the bounds should change.

This is a big change @ben-lu-uw , thank you for your hard work on it. Merging.

prushforth reviewed Oct 14, 2021

View reviewed changes

Malvoz reviewed Oct 22, 2021

View reviewed changes

src/mapml-viewer.js Outdated Show resolved Hide resolved

ben-lu-uw marked this pull request as ready for review October 25, 2021 17:44

ben-lu-uw requested review from prushforth, Malvoz and ahmadayubi October 25, 2021 17:45

Malvoz requested changes Oct 25, 2021

View reviewed changes

src/mapml-viewer.js Outdated Show resolved Hide resolved

ahmadayubi reviewed Oct 25, 2021

View reviewed changes

ben-lu-uw marked this pull request as draft October 26, 2021 03:36

ben-lu-uw requested a review from ahmadayubi October 27, 2021 13:51

prushforth mentioned this pull request Oct 30, 2021

Implement custom non-visual navigation grids #552

Open

ben-lu-uw force-pushed the announce_zoom_and_move branch from 505c808 to dd5dd10 Compare November 10, 2021 01:08

ben-lu-uw mentioned this pull request Nov 15, 2021

Differentiating between the user and code setting focus #577

Open

ben-lu-uw force-pushed the announce_zoom_and_move branch from f9f1362 to ddbe48a Compare November 17, 2021 20:35

ben-lu-uw added 5 commits November 17, 2021 21:50

Readd output CSS

9afeeff

Implement Ahmad's M.options.announceMovement

c38162d

Add new event mapfocused

ed8ca15

Fix tests

357b403

ben-lu-uw force-pushed the announce_zoom_and_move branch from a40b530 to 357b403 Compare November 18, 2021 02:55

Add announce movement locales

d96a354

ben-lu-uw marked this pull request as ready for review November 18, 2021 16:13

prushforth approved these changes Nov 23, 2021

View reviewed changes

prushforth merged commit ddf4db8 into Maps4HTML:main Nov 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Announce zoom and move and implementing map boundaries #533

Announce zoom and move and implementing map boundaries #533

ben-lu-uw commented Oct 14, 2021 •

edited

Loading

ahmadayubi commented Oct 14, 2021

prushforth Oct 14, 2021

prushforth Oct 14, 2021

prushforth Oct 14, 2021

prushforth Oct 14, 2021

ahmadayubi Oct 25, 2021

ahmadayubi Oct 25, 2021

ahmadayubi Oct 25, 2021

ahmadayubi Oct 25, 2021

ben-lu-uw commented Oct 26, 2021 •

edited

Loading

prushforth commented Nov 4, 2021

Malvoz commented Nov 4, 2021

prushforth left a comment

Announce zoom and move and implementing map boundaries #533

Announce zoom and move and implementing map boundaries #533

Conversation

ben-lu-uw commented Oct 14, 2021 • edited Loading

ahmadayubi commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ben-lu-uw commented Oct 26, 2021 • edited Loading

prushforth commented Nov 4, 2021

Malvoz commented Nov 4, 2021

prushforth left a comment

Choose a reason for hiding this comment

ben-lu-uw commented Oct 14, 2021 •

edited

Loading

ben-lu-uw commented Oct 26, 2021 •

edited

Loading