feat: add optional prop to set aria-label on OL canvas element in shadow root
#445
Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -629,6 +632,10 @@ export class MyMap extends LitElement { | |||
| }); | |||
| } | |||
|
|
|||
| // Add an aria-label to the overlay canvas for accessibility | |||
| const olCanvas = this.renderRoot?.querySelector("canvas.ol-fixedoverlay"); | |||
There was a problem hiding this comment.
There's two canvas elements rendered by OL in the shadow root, but I've only been able to successfully select this one - which does differ than the code reference given in our report:
Current code ref(s):
#property-information-map > div > div.ol-unselectable.ol-layers > div:nth-child(2) > canvas
So,
- Am I missing something obvious to be able to select the
.ol-unselectable .ol-layers canvaselement? - If that canvas is actually unselectable, are we still happy applying this proposed fix to this canvas and seeing how it re-tests?
There was a problem hiding this comment.
This is a good point to flag and I think I know why you're getting inconsistent results.
My understanding is that OpenLayers doesn't generate either a single canvas, or canvas per-layer, but rather adds additional canvases as needed for memory/rendering.
I'm trying to find some sort of reference to this online and nothing yet to confirm this in writing!
Looking through OpenLayers examples and inspecting the DOM you can see maps with multiple layers on a single canvas, and some spread over multiple canvases.
What might be best here is trying to select all canvas elements (querySelectorAll() I think) and appending ${ariaLabelOlFixedOverlay}-${i}?
There was a problem hiding this comment.
Dev call resolution here:
- We don't want to label all & overread (even if we could successfully select all)
- By targeting
fixedoverlay, we are targetting the outermost canvas which will hopefully still achieve same result as original suggestion - It's a usability issue, not A-level - so let's keep it timeboxed!
| @@ -629,6 +632,10 @@ export class MyMap extends LitElement { | |||
| }); | |||
| } | |||
|
|
|||
| // Add an aria-label to the overlay canvas for accessibility | |||
| const olCanvas = this.renderRoot?.querySelector("canvas.ol-fixedoverlay"); | |||
There was a problem hiding this comment.
This is a good point to flag and I think I know why you're getting inconsistent results.
My understanding is that OpenLayers doesn't generate either a single canvas, or canvas per-layer, but rather adds additional canvases as needed for memory/rendering.
I'm trying to find some sort of reference to this online and nothing yet to confirm this in writing!
Looking through OpenLayers examples and inspecting the DOM you can see maps with multiple layers on a single canvas, and some spread over multiple canvases.
What might be best here is trying to select all canvas elements (querySelectorAll() I think) and appending ${ariaLabelOlFixedOverlay}-${i}?
Adds an optional string prop
ariaLabelOlFixedOverlaywhich sets anaria-labelon thecanvaselement rendered in the shadow root.Per page 98 of recent audit:
References / possible gotchas:
ariaLabelprop on the container div element break: remove ariaLabel property #86