Add chart component #4301

andysellick · 2024-10-11T12:53:09Z

What

Adds a chart component.

Based on a component from content-data-admin originally added here: alphagov/content-data-admin#9

TODO:

check the ONS guidance on accessible graphs to see what improvements we can make
fix the console error when the graph is hovered

Why

Visual Changes

New component:

Trello card: https://trello.com/c/gvLYQCUp/57-build-statistics-charts

AshGDS

Code wise it looks good, but I just had a few questions about the mobile styles, as the Axis Labels seem to disappear on mobile, and when you click on a data point, it covers other data points so you can't click on them. Are we OK with this on mobile?

AshGDS · 2024-10-15T14:59:29Z

Also, I appreciate that this was a difficult component to take on, so thanks for doing this 👍

jon-kirwan

It looks good to me. I've tested across various screens / devices, with VoiceOver, with JS disabled and seems to work well for me. I just added two small comments / questions.

app/views/govuk_publishing_components/components/_chart.html.erb

app/views/govuk_publishing_components/components/docs/chart.yml

andysellick · 2024-10-16T08:23:21Z

Code wise it looks good, but I just had a few questions about the mobile styles, as the Axis Labels seem to disappear on mobile, and when you click on a data point, it covers other data points so you can't click on them. Are we OK with this on mobile?

Yeah, the axis labels disappearing as the graph gets narrower is a bit unhelpful, although this seems to be the behaviour of the graph so I think we're stuck with it. I think the problem might be that (certainly with some data) it wouldn't be possible to fit the labels (certainly for the Y axis) in with the graph on mobile - imagine if the Y scale was 0 -> 1,000,000,000. Hopefully the presence of the tooltips will help. I'm also anticipating that once we start using this component we might find some alternative ways of configuring it to better suit what it gets used for.

On the point about the tooltip, I've just pushed a commit to switch to using HTML tooltips (instead of SVG). It seems now that it's much easier to dismiss the tooltip by tapping elsewhere on the page, so hopefully this has improved things.

AshGDS

Thanks @andysellick - the experience on mobile is a lot better with the new tooltips.

Code wise everything looks good 👍 so I would approve the PR on the basis of code, but had a few questions:

It seems that you can't right click the element - is that intentional?
The charts seem to have an aria-label that is A chart. - is the lack of customisation for this intentional? The table has an aria label as well.
The HTML seems to have col" attributes in it, which isn't a valid attribute - is this a chartkick thing we can ignore, or a bug?
The ONS charts fallback to an image when JS is disabled, so that may be worth noting for the future improvements to this component.

AshGDS · 2024-10-16T08:54:44Z

spec/components/chart_spec.rb

+    assert_select ".govuk-table__cell--numeric", 26
+    assert_select ".govuk-table__header", 14
+    assert_select "td:first", text: "5"
+    assert_select "td:last", text: "121"


Just wondering for these tests if we should be testing data points in the middle as well (I guess it's better to be overly safe when it comes to data)

Sure, have added.

AshGDS · 2024-10-16T10:47:42Z

app/views/govuk_publishing_components/components/_chart.html.erb

+  table_id = "table-id-#{SecureRandom.hex(4)}"
+
+  require "chartkick"
+  Chartkick.options[:html] = '<div id="%{id}"><noscript><p class="govuk-body">Our charts are built using JavaScript but all the data is also available in the table below.</p></noscript></div>'


Also, does this text need editorial approval?

Possibly. If it's alright with you I think we'll look at that later when we're reviewing the pages as a whole.

andysellick · 2024-10-16T10:56:16Z

It seems that you can't right click the element - is that intentional?

It's not intentional from us, but appears to be intentional in Google charts, see this stack overflow for info: https://stackoverflow.com/questions/20266832/why-google-charts-are-without-right-click-menu

The charts seem to have an aria-label that is A chart. - is the lack of customisation for this intentional? The table has an aria label as well.

This is generated automatically by Google charts. It doesn't seem to be possible to directly customise this text - there are some complicated JS solutions online, which I'm hesitant to implement at this point as I think there's not much more helpful information we can provide at this level - the chart should already have a title, and an accessible description.

Interestingly while digging into this I realised the component has two copies of the table - the one that we create inside the details component, and another one that Google charts automatically creates and appends after the chart. It's visually hidden, so still readable by screenreaders. I'll make a note to see if I can hide it somehow.

The HTML seems to have col" attributes in it, which isn't a valid attribute - is this a chartkick thing we can ignore, or a bug?

I'm not sure, where are these exactly?

The ONS charts fallback to an image when JS is disabled, so that may be worth noting for the future improvements to this component.

Noted, thanks.

AshGDS · 2024-10-16T11:11:58Z

@andysellick No worries - thanks for looking into those.

I don't think the col" thing is actually an issue - instead I think it is a bug with the W3C HTML validator I used on the chart, my bad.

AshGDS

Nice work 👍

- copy of code from content-data-admin, with the following adjustments - added explicit `require "chartkick"` in template to prevent error - added template calls to include required stylesheets for chart, table and details component - modified output of keys to simply output key, previously was doing `key.to_formatted_s(:table_format)` expecting a date, to convert it into a slightly more human readable format, but this was erroring, so simply outputting the key, adjusted test to match - renamed class from `app-c-` to `gem-c-` - replaced instances of 14 font to 16, in line with Design System guidance - reordered some of the Sass so `@include` lines come at the end of declarations, to avoid Sass compilation warnings - modified test slightly to fit format of the gem tests

- add and style axes labels and legends - remove some specific styles that were overriding normal table styles - simplify CSS, chart height appeared to be duplicated - increase point size - remove specifics for line colour and style, let the chart decide

- clean up, reorder and remove redundant code - fix table vertical option - add heading element - minimise and simplify component options - add option for a chart overview description - add option to hide legend - remove unnecessary use of govspeak to wrap a visually hidden element - remove unneed styles - add tests, expand and clean up documentation

- using Google charts instead - chart.js is the default and produces nice responsive canvas graphs but provides far fewer options for customising important things like the font size and style of axes

- tooltip now uses the HTML option, which for some reason means it's easier to dismiss on mobile by tapping elsewhere on the page - also improve the styling, both in terms of code and appearance

- move accessibility text into the YML file - clean up the doc file a little and mark component as experimental

- Google charts automatically appends a tabular version of the graph data in a visually hidden element after the chart, but this is redundant and a bit confusing as we already have a table of the data displayed underneath the chart for all users - there seems to be no configuration option for controlling this automatic table so I've used some slightly cludgy CSS to display none it, thereby hiding it from all users including screen readers

- adds the shared helper to control margin bottom - adds the component wrapper helper because I initially remembered it wrong and thought it did margin bottom but then remembered it was the shared helper that did that but actually having the component wrapper helper on this component is no bad thing - adds an option for a download link

- smoothing distorts the graph by implying that change between data points is smooth when it isn't

govuk-ci temporarily deployed to components-gem-pr-4301 October 11, 2024 12:53 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 11, 2024 14:21 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 11, 2024 15:05 Inactive

andysellick force-pushed the add-chart-component branch from 5c9067a to 763f019 Compare October 15, 2024 07:18

govuk-ci temporarily deployed to components-gem-pr-4301 October 15, 2024 07:19 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 15, 2024 14:06 Inactive

andysellick force-pushed the add-chart-component branch from c8dec1d to 939dae8 Compare October 15, 2024 14:08

govuk-ci temporarily deployed to components-gem-pr-4301 October 15, 2024 14:08 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 15, 2024 14:11 Inactive

andysellick changed the title ~~[WIP] [DO NOT MERGE] Add chart component~~ [DO NOT MERGE] Add chart component Oct 15, 2024

AshGDS reviewed Oct 15, 2024

View reviewed changes

jon-kirwan reviewed Oct 15, 2024

View reviewed changes

app/views/govuk_publishing_components/components/_chart.html.erb Outdated Show resolved Hide resolved

app/views/govuk_publishing_components/components/docs/chart.yml Outdated Show resolved Hide resolved

andysellick marked this pull request as ready for review October 16, 2024 07:51

govuk-ci temporarily deployed to components-gem-pr-4301 October 16, 2024 08:16 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 16, 2024 08:43 Inactive

AshGDS reviewed Oct 16, 2024

View reviewed changes

govuk-ci temporarily deployed to components-gem-pr-4301 October 16, 2024 12:25 Inactive

govuk-ci temporarily deployed to components-gem-pr-4301 October 16, 2024 12:48 Inactive

andysellick force-pushed the add-chart-component branch from 6a778c4 to dd89779 Compare October 16, 2024 12:53

govuk-ci temporarily deployed to components-gem-pr-4301 October 16, 2024 12:54 Inactive

andysellick force-pushed the add-chart-component branch from dd89779 to 1d41ea9 Compare October 17, 2024 09:35

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 09:35 Inactive

andysellick force-pushed the add-chart-component branch from 1d41ea9 to e090912 Compare October 17, 2024 09:45

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 09:46 Inactive

andysellick force-pushed the add-chart-component branch from e090912 to 4aa9882 Compare October 17, 2024 09:47

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 09:47 Inactive

AshGDS approved these changes Oct 17, 2024

View reviewed changes

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 09:54 Inactive

andysellick changed the title ~~[DO NOT MERGE] Add chart component~~ Add chart component Oct 17, 2024

andysellick added 11 commits October 17, 2024 11:52

Add translation file entries

b1a40ef

Make example data more detailed

f7851ee

Improve chart appearance

d6d6c8e

- add and style axes labels and legends - remove some specific styles that were overriding normal table styles - simplify CSS, chart height appeared to be duplicated - increase point size - remove specifics for line colour and style, let the chart decide

Remove chart.js

c799893

- using Google charts instead - chart.js is the default and produces nice responsive canvas graphs but provides far fewer options for customising important things like the font size and style of axes

Improve tooltip

285605c

- tooltip now uses the HTML option, which for some reason means it's easier to dismiss on mobile by tapping elsewhere on the page - also improve the styling, both in terms of code and appearance

Improve text

7688f20

- move accessibility text into the YML file - clean up the doc file a little and mark component as experimental

Don't smooth graph lines

061745c

- smoothing distorts the graph by implying that change between data points is smooth when it isn't

andysellick force-pushed the add-chart-component branch from 0583c75 to 061745c Compare October 17, 2024 10:52

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 10:52 Inactive

Update changelog

5645d4a

govuk-ci temporarily deployed to components-gem-pr-4301 October 17, 2024 10:53 Inactive

andysellick merged commit 76702a0 into main Oct 17, 2024
12 checks passed

andysellick deleted the add-chart-component branch October 17, 2024 10:56

andysellick mentioned this pull request Oct 17, 2024

Version 44.4.0 #4310

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add chart component #4301

Add chart component #4301

andysellick commented Oct 11, 2024 •

edited

Loading

AshGDS left a comment

AshGDS commented Oct 15, 2024

jon-kirwan left a comment

andysellick commented Oct 16, 2024

AshGDS left a comment

AshGDS Oct 16, 2024

andysellick Oct 17, 2024

AshGDS Oct 16, 2024

andysellick Oct 17, 2024

andysellick commented Oct 16, 2024

AshGDS commented Oct 16, 2024 •

edited

Loading

AshGDS left a comment

Add chart component #4301

Add chart component #4301

Conversation

andysellick commented Oct 11, 2024 • edited Loading

What

Why

Visual Changes

AshGDS left a comment

Choose a reason for hiding this comment

AshGDS commented Oct 15, 2024

jon-kirwan left a comment

Choose a reason for hiding this comment

andysellick commented Oct 16, 2024

AshGDS left a comment

Choose a reason for hiding this comment

AshGDS Oct 16, 2024

Choose a reason for hiding this comment

andysellick Oct 17, 2024

Choose a reason for hiding this comment

AshGDS Oct 16, 2024

Choose a reason for hiding this comment

andysellick Oct 17, 2024

Choose a reason for hiding this comment

andysellick commented Oct 16, 2024

AshGDS commented Oct 16, 2024 • edited Loading

AshGDS left a comment

Choose a reason for hiding this comment

andysellick commented Oct 11, 2024 •

edited

Loading

AshGDS commented Oct 16, 2024 •

edited

Loading