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.

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

Fix some failed results from Web map tools WCAG 2.1 evaluation #9991

Merged
merged 9 commits into from
Oct 5, 2020

Conversation

tristen
Copy link
Member

@tristen tristen commented Sep 18, 2020

Addresses some Level A and Level AA failures as outlined in https://github.com/Malvoz/web-maps-wcag-evaluation/blob/ac797a3e5fe984db27662c27ae05077e7b22026b/README.md

Level A

In https://github.com/Malvoz/web-maps-wcag-evaluation#131-info-and-relationships-level-a

The "Zoom out" control's disabled state cannot be programmatically determined.

Change Screenshot
Adds aria-hidden attribute when min/max controls trigger disabled states disabled-state

In https://github.com/Malvoz/web-maps-wcag-evaluation#211-keyboard-level-a

Control to display attribution and feedback links is not keyboard accessible.

Change Screenshot
When attribution is compact, the icon now acts as a toggle button to open and close attribution contents. This changes the behavior of it being triggered on hover but I think its a more sensible/predictable change to make. attribution

In https://github.com/Malvoz/web-maps-wcag-evaluation#412-name-role-value-level-a

The "map component" (, which acts as a control to both zoom and pan the map display) is missing role.

Added role:region to the canvas container.

Control to display attribution and feedback links is missing name and role.

Added role:list to the .mapboxgl-ctrl-attrib-inner container. This also requires an upstream change to the TileJSON doc to add role:listitem

Level AA

In https://github.com/Malvoz/web-maps-wcag-evaluation#247-focus-visible-level-aa

1 tab stop without focus indicator.

This is addressed by turning the compact attribution link into a button toggle with focus.

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Addresses some Level A and Level AA accesibility recommendations</changelog>

@ryanhamley
Copy link
Contributor

That's a really cool doc that we can reference! This is exciting work.

@tristen would you be willing to take a look at #9774 as part of this work? It's an attempt to make markers more accessible and focusable. I'm not sure it's quite ready, but it would be helpful to get a second set of eyes on it.

Also cc @malwoodsantoro who has been working on an accessibility plugin

src/ui/map.js Outdated Show resolved Hide resolved
Comment on lines +118 to +120
{key: 'owner', value: this.styleOwner},
{key: 'id', value: this.styleId},
{key: 'access_token', value: this._map._requestManager._customAccessToken || config.ACCESS_TOKEN}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not a necessary change but I just came across this string inconsistency.

@tristen tristen marked this pull request as ready for review September 21, 2020 23:45
@andrewharvey
Copy link
Collaborator

Cool looks like this would close #3959

@asheemmamoowala asheemmamoowala requested review from ryanhamley and removed request for asheemmamoowala September 30, 2020 17:42
Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

Overall this looks really great. I just have a few minor suggestions.

src/ui/control/navigation_control.js Outdated Show resolved Hide resolved
this._innerContainer = DOM.create('div', 'mapboxgl-ctrl-attrib-inner', this._container);
this._innerContainer.setAttribute('role', 'list');
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment about this feature is Control to display attribution and feedback links is missing name and role.. Should we add a name attribute to this feature as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, name and role for the feedback links themselves. I have a PR in a branch to address that in the TileJSON. The attribution control just receives these as strings and appends them in so it felt right to do it upstream.


.mapboxgl-ctrl-attrib.mapboxgl-compact::after {
content: '';
.mapboxgl-ctrl-attrib-button {
Copy link
Contributor

Choose a reason for hiding this comment

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

Screen Shot 2020-10-01 at 11 44 03 AM

I think the compact attribution could use a bit more space between the text and the button. When the button is focused, which it is when the control is first maximized after clicking it or hitting Enter, the blue focus ring abuts the text pretty tightly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Increased padding in 67282f4 to address this!

Screen Shot 2020-10-04 at 1 27 37 PM

@tristen tristen requested a review from ryanhamley October 5, 2020 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make AttributionControl with compact option accessible
4 participants