EES-6580 indicate the presence of the map to screen reader users by adding accessibility attributes to the map component #6361
Conversation
…rs so that the screen reader reads the alternative text when a user is interacting with a map.
mmoalin
changed the title
|
Looking at the Jira ticket, we also need to amend the legend to use different markup - a definition list ( Just be aware when changing this that some UI tests may need amending after this too (not 100% sure on that but I would guess so!) |
src/explore-education-statistics-common/src/modules/charts/components/MapBlock.tsx
Outdated
Show resolved
Hide resolved
src/explore-education-statistics-common/src/modules/charts/components/MapBlock.tsx
Outdated
Show resolved
Hide resolved
src/explore-education-statistics-common/src/modules/charts/components/MapBlock.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
I'm really not sure that adding the named colours is adding any benefit to users - as there are so many possible colours, the resulting named colours are kind of meaningless. Attached a screenshot to show what the output is and I don't think it's helpful in any way.
Having said that, I can't think of a better way of naming them apart from possibly getting the 'base' colour and just calling each one 'shade {i of x} of {base colour}'
Or leaving the named colours out completely.
As we've now had to add another dependency which will add JS weight and time for rendering the page, for something that I don't think is helpful...
Maybe we need to get others' opinions. Or if we're able to query with DAC, or research online...
There was a problem hiding this comment.
Minor, could you put these in the right place (alphabetical ordered)? Will just help when scanning/managing deps as there are many of them and is standard practise. Ta!
| width: 20px; | ||
| } | ||
|
|
||
| .srOnly { |
There was a problem hiding this comment.
We shouldn't need to define this class here - we already have visually hidden text as a component you can use instead of a span className="srOnly"
<VisuallyHidden>text here</VisuallyHidden>
| }} | ||
| /> | ||
| </dt> | ||
| <span className={styles.srOnly}> |
There was a problem hiding this comment.
2 points:
- This span (change to VisuallyHidden as per above comment) needs to go inside the
<dt> - I don't think we need the colour to be written in hex format particularly, the important thing is to write the value of the colour so it is not only visually represented. So probably fine to leave in rgb format if that's what it's saved/returned as.
| function componentToHex(c: number): string { | ||
| const hex = c.toString(16); | ||
| return hex.length === 1 ? `0${hex}` : hex; | ||
| } | ||
|
|
||
| function rgbaToHex(rgba: string): string { | ||
| const match = rgba.match( | ||
| /^rgba?\(\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(\d{1,3})(?:\s*,\s*([\d.]+))?\s*\)$/, | ||
| ); | ||
| if (!match) { | ||
| throw new Error('Invalid RGBA color format'); | ||
| } | ||
|
|
||
| const [, r, g, b, a] = match; | ||
| const rParsed = parseInt(r, 10); | ||
| const gParsed = parseInt(g, 10); | ||
| const bParsed = parseInt(b, 10); | ||
|
|
||
| const clamp = (n: number, min: number, max: number) => | ||
| Math.min(max, Math.max(min, n)); | ||
|
|
||
| const red = clamp(Math.round(rParsed), 0, 255); | ||
| const green = clamp(Math.round(gParsed), 0, 255); | ||
| const blue = clamp(Math.round(bParsed), 0, 255); | ||
|
|
||
| let alphaHex = ''; | ||
|
|
||
| if (a !== undefined) { | ||
| const alpha = clamp(parseFloat(a), 0, 1); | ||
| alphaHex = componentToHex(Math.round(alpha * 255)); | ||
| } | ||
|
|
||
| return `#${componentToHex(red)}${componentToHex(green)}${componentToHex( | ||
| blue, | ||
| )}${alphaHex}`; | ||
| } |
There was a problem hiding this comment.
See above comment, but I don't think we need these two functions
| ); | ||
| } | ||
|
|
||
| function hexToColourName(rgba: string): string | false { |
There was a problem hiding this comment.
Don't think hexToColourName is the right name if we're converting from rgba!
| } | ||
|
|
||
| function hexToColourName(rgba: string): string | false { | ||
| const name = ColorNamer(rgba, { pick: ['ntc', 'pantone'] }); |
There was a problem hiding this comment.
Out of interest, any reason we're picking these two lists?
NIT but the variable could be called names as it's got multiple properties/lists
| const pantone = name.pantone[0] ? name.pantone[0].name : false; | ||
| const ntc = name.ntc[0] ? name.ntc[0].name : false; | ||
| if (ntc) return ntc; | ||
| if (pantone) return pantone; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Think we could make this a bit simpler instead of explicitly passing false everywhere e.g.
| Key to {heading} | ||
| </h3> | ||
| <ul className="govuk-list" data-testid="mapBlock-legend"> | ||
| <dl className="govuk-list" data-testid="mapBlock-legend"> |
There was a problem hiding this comment.
After converting to the <dl> we're now missing margin beneath each item - it was applied to li elements, can be added in manually to our styles.legend class.
There was a problem hiding this comment.
Just checking you've ran the UI tests after this change? I know we're querying for legend items - hopefully they shouldn't break because of it but just want to make sure
3 participants
This PR adds an alternative text to the map component in order to indicate the presence of the map component to screen reader users.
I couldn't find native APIs from leaflet or react-leaflet to use and so this is a workaround that adds aria-label attribute to the map component after it renders so that the screen reader reads the alternative text when a user is interacting with a map.