fix: usability improvements#442
Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Looks like a great improvement from a visual pov! One small suggestion, as the styles are written with SCSS, set a variable (i.e |
|
Thanks @ianjon3s - good spot, now updated! I'm out of practice with stylesheets - always appreciate these tips 🙂 |
ianjon3s
left a comment
ianjon3s
left a comment
There was a problem hiding this comment.
Approving from a non-typescript perspective (@DafyddLlyr may want to add other comments), will be good to see a working example of this to test focus states etc.
DafyddLlyr
left a comment
DafyddLlyr
left a comment
There was a problem hiding this comment.
Great to see tests added as always!
A few suggestions here - possibly something you've already tried.
|
|
||
| // Apply tabindexes to OL Controls & Attribution links for accessibility (container div has starting tabindex="1") | ||
| const controlsLength = olControls?.length; | ||
| olControls?.forEach((node, i) => |
There was a problem hiding this comment.
I think that the order of these in the DOM is determined by their order in the collection of map controls.
We set up defaults, then based on props, append new ones. This is why the variable ones come after the defaults (e.g. attribution).
Totally fair if you've already explored this one, but it might be worth trying to just manage this via the order they're added to the map controls? Main motivations here would be simplicity (arguable depending on how that works out!), and also the general recommendation to to not use tabindex unless totally required -
Warning: You are recommended to only use 0 and -1 as tabindex values. Avoid using tabindex values greater than 0 and CSS properties that can change the order of focusable HTML elements (Ordering flex items). Doing so makes it difficult for people who rely on using keyboard for navigation or assistive technology to navigate and operate page content. Instead, write the document with the elements in a logical sequence.
Source: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex
There was a problem hiding this comment.
This is sorted now by re-ordering, no longer having to rely on positive tab indexes!
// Re-order overlay elements so that OL Attribution is final element
// making OL Controls first in natural tab order for accessibility
const olAttribution = this.renderRoot?.querySelector(".ol-attribution") as Node;
const olOverlay = this.renderRoot?.querySelector(".ol-overlaycontainer-stopevent");
olOverlay?.append(olAttribution);
There was a problem hiding this comment.
Great solution - super simple 👍
3 participants
A number of styling tweaks based on feedback from Planx's most recent accessibility audit:
staticModeis set and attributions are not collapsedFuture scope / followup:
aria-labelon canvas element in shadow dom, not container div rendered by Lit