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.

Sign up for GitHub

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

Adding tabbing navigation and accessibility to map #270

Merged
merged 25 commits into from
Feb 10, 2021
Merged

Adding tabbing navigation and accessibility to map #270

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
831e5fb
Allow features to be focused
ahmadayubi Jan 27, 2021
ba5078b
Adds crosshair, shows on keyboard movement
ahmadayubi Jan 28, 2021
1c75af6
Shows crosshair on tab
ahmadayubi Jan 28, 2021
8d82998
Only run on moveend if there are queryable layers
ahmadayubi Jan 28, 2021
9d9029e
Revert listener change
ahmadayubi Jan 29, 2021
97b811d
Adds tests for keyboard interaction
ahmadayubi Jan 29, 2021
5812169
Merge branch 'master' into featureFocus
prushforth Feb 1, 2021
f7f1497
Merge branch 'master' into featureFocus
ahmadayubi Feb 1, 2021
64476e6
Merge branch 'master' into featureFocus
ahmadayubi Feb 2, 2021
f5911b0
Remove close button from feature popup
ahmadayubi Feb 2, 2021
61a83d4
Add bypass navigation
ahmadayubi Feb 3, 2021
edc2cf0
Query popup fix
ahmadayubi Feb 3, 2021
a8d82fd
Merge branch 'master' into featureFocus
prushforth Feb 3, 2021
525d68e
Merge branch 'master' into featureFocus
prushforth Feb 3, 2021
2ed2749
Merge branch 'master' into featureFocus
prushforth Feb 3, 2021
3147842
Add feature count, move controls to bottom
ahmadayubi Feb 4, 2021
783d521
Merge branch 'featureFocus' of https://github.com/ahmadayubi/Web-Map-…
ahmadayubi Feb 4, 2021
8bc75ce
Test update to consider skip buttons
ahmadayubi Feb 4, 2021
69478aa
Test update
ahmadayubi Feb 4, 2021
7326fa1
Adds next and previous focus buttons
ahmadayubi Feb 4, 2021
19aaf5e
Add tests for keyboard interaction
ahmadayubi Feb 5, 2021
d227589
Rename variables and add comments
ahmadayubi Feb 5, 2021
8b3b1ef
Remove handlers on close
ahmadayubi Feb 5, 2021
0f69555
Merge branch 'master' into featureFocus
ahmadayubi Feb 10, 2021
a6c7f8d
Merge branch 'master' into featureFocus
prushforth Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/mapml/layers/MapLayer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export var MapMLLayer = L.Layer.extend({
if (properties) {
var c = document.createElement('div');
c.insertAdjacentHTML('afterbegin', properties.innerHTML);
geometry.bindPopup(c, {autoPan:false});
geometry.bindPopup(c, {autoPan:false, closeButton: false});
Copy link
Member

Choose a reason for hiding this comment

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

Why are close buttons not useful in feature's popups?

Copy link
Member

Choose a reason for hiding this comment

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

I think @shepazu mentioned they might be problematic, or it might have been me misinterpreting something...

Copy link
Member Author

Choose a reason for hiding this comment

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

It was suggested that it be removed, should it be added back?

Copy link
Member

Choose a reason for hiding this comment

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

I think having a close button is fine, with the following caveats:

  • the popup should not be a modal (that is, it shouldn't be a focus trap)
  • if possible, we shouldn't force screen reader users to "close" each feature popup to move on to another feature

This seems like somewhat competing experiences between voice-control and screen-reader users, in exposing the right amount of detail with the fewest possible number of forced interactions. My preference would be to have the popup not exposed to screen readers on focus, but only read the accessible name (the popup "title"), with notification of additional information (e.g. aria-expanded) to access the other popup details only on demand (at which point they might encounter the close button, but that's fine, because they chose to expand it). For voice-control users, I think having the popup on focus should be good, with scrolling for long details, and a close button.

Does this seem right to you, @Malvoz?

Copy link
Member

@Malvoz Malvoz Feb 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think having a close button is fine, with the following caveats:

  • the popup should not be a modal (that is, it shouldn't be a focus trap)

The popups are non-modal, so focus traps are (mostly*) not an issue.

*when the popup for the last feature in the sequence is opened tabbing is currently trapped to some extent (shift + tab can be used to move back to the previous feature):

Keyboard.Interaction.Test.-.Google.Chrome.2021-02-08.11-06-.mp4

if possible, we shouldn't force screen reader users to "close" each feature popup to move on to another feature

Because focus (generally) isn't trapped inside popups users don't have to explicitly close popups before moving on to other features/focusable elements.

This seems like somewhat competing experiences between voice-control and screen-reader users, in exposing the right amount of detail with the fewest possible number of forced interactions.

I'm not super familiar with voice-control so I'm not sure about how there are competing experiences here. I do know that some people use voice-control to move their cursor around to interact with elements, in that case I think a close button could be appreciated if they just wanted to close a popup without opening a new one.

My preference would be to have the popup not exposed to screen readers on focus, but only read the accessible name (the popup "title"), with notification of additional information (e.g. aria-expanded) to access the other popup details only on demand (at which point they might encounter the close button, but that's fine, because they chose to expand it).

Yes I think we agreed that popups should not be opened (and thus exposed to ATs) when a feature recieves focus, and that each feature itself should be labelled for discernibility. (discussion from #270 (comment))

I agree that features that are interactive and have popups should expose the state of the popup using aria-expanded.

}
}
});
Expand Down Expand Up @@ -123,7 +123,7 @@ export var MapMLLayer = L.Layer.extend({
if (properties) {
var c = document.createElement('div');
c.insertAdjacentHTML('afterbegin', properties.innerHTML);
geometry.bindPopup(c, {autoPan:false});
geometry.bindPopup(c, {autoPan:false, closeButton: false});
}
}
}).addTo(map);
Expand Down
2 changes: 1 addition & 1 deletion src/mapml/layers/TemplatedFeaturesLayer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export var TemplatedFeaturesLayer = L.Layer.extend({
// need to parse as HTML to preserve semantics and styles
var c = document.createElement('div');
c.insertAdjacentHTML('afterbegin', properties.innerHTML);
geometry.bindPopup(c, {autoPan:false});
geometry.bindPopup(c, {autoPan:false, closeButton: false});
}
});
}
Expand Down