UI: Add some simple accessibility labels for line charts

[DingoEatingFuzz]

SVGs by default are either ignored entirely by screen readers or are read in an excruciating and unhelpful detail.

By adding some attributes and helpful labels, these charts become much more useful for people with visual impairments.

Includes

1. Labeling svg charts as role=img
2. Title and description fields for line charts
3. aria-labelledby on svgs to associate the title and description fields in case the screen reader doesn't do that automatically.
4. Use case specific title and label for the stats time series chart
5. A longForm variation of the format-duration helper that ensures times are read correctly.

[johncowen]

Hey!

Right so this one was a little difficult for me to try out! :) Not sure exactly what is expected, but it does read out the text (using VoiceOver on Safari and also Chrome which seem to work slightly differently)

It would be difficult for me to approve/not approve anything here based on usage, as I'm really not sure what is expected - VoiceOver seems to repeat things a lot (I think this is the excruciating bit you mention 😂 ). Would be good to chat with you to see what you are aiming for here. Happy to approve functionality-wise if it does what you are aiming for/expecting.

Code-wise, 1st little nit is you use 2 forms of the phrase 'time series', one with a hyphen and one without (smallest nit ever!). Apart from that the format-duration code seems quite involved, maybe worthwhile adding some doc at the top explaining the problem its solving, plus a few more inline comments explaining what it's doing where, whatever you think would be useful though.

I think overall I'd have some questions over the fact that you have used the plural forms as the name in your allUnits data, and then you are using a pluralize method to pluralize the longSuffix? I've also just noticed you seem to be pluralizing the 'short form' of the suffix, so 'ms' etc? Maybe I've misunderstood that. The more I look at this file the more I wonder about it, I think this would be one of those places we'd differ - but I'm happy if you are happy with it yourself? It's tested and it looks like it does what it says on the tin - a bit more documentation would also help.

Anyway, sorry not the greatest review in the world :(

Apart from the above, I'm not actually seeing any lines drawn on the graphs?

Feel free to shout me to go through if that would help.

[DingoEatingFuzz]

I think overall I'd have some questions over the fact that you have used the plural forms as the name in your allUnits data, and then you are using a pluralize method to pluralize the longSuffix?

I suppose I could use the name field instead of introducing the longSuffix method, but the name field was mostly to map to moment method names. All suffix and longSuffix values are consistently singular.

I've also just noticed you seem to be pluralizing the 'short form' of the suffix, so 'ms' etc? Maybe I've misunderstood that.

Short forms don't get pluralized as dictated by the pluralizable: false flag.

The more I look at this file the more I wonder about it, I think this would be one of those places we'd differ - but I'm happy if you are happy with it yourself?

It's not my favorite code, but it's also a sticky situation. Can you describe what is it you don't like about it?

It's tested and it looks like it does what it says on the tin - a bit more documentation would also help.

There are inline comments, but I'll add some more for the new pluralizeUnits refactor that involves longForm.

[johncowen]

I suppose I could use the name field instead of introducing the longSuffix method, but the name field was mostly to map to moment method names. All suffix and longSuffix values are consistently singular.

Yeah don't stress, it's just a thing I noticed and maybe part of the reason why both of us probably think this isn't 'our favorite'.

Can you describe what is it you don't like about it?

It's hard to put my finger on. Kind of why I asked if you were happy, I had a feeling it wasn't your favorite either. I think it was just a general feeling of 'there's a lot going on here, and reading through it a few times I'm still not exactly sure'. No biggie, maybe come back at a later date and see if can be improved at all.

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.