-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Infographics and tables #107
Conversation
Hi Heydon! @micmath asked me to look at this. I've just loaded it up locally and had a play: Reference implementation - Data Tables
Article - Data Tables
I hope this helps. |
@simonsinclair Hi Simon! Thanks for taking a look. Did you only try the table reference implementation, and not the infographics one?
Damned Safari! It was a bug with
Good spot! Done.
So... in research (not at the BBC), it was found that folks weren't sure whether buttons should display the current state or the state to be attained after the next button press. That is, changes in state were confusing. For now, I have the buttons just say "pressing this will sort the column" to avoid the state complexity issues. The change in state is visible simply because the column is seen to sort + ARIA is provided on the parent |
Morning, I just reviewed Infographics. (Why am I not seeing either of these articles on the GEF Index?) Reference Implementation - Infographics
Article - Infographics
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great @Heydon! I've noted a few minor tweaks, but am loving the information! 😍
``` | ||
|
||
To make this element scrollable by keyboard, it must first be focusable. This requires the `tabindex="0"` attribution. For screen reader users, this newly interactive element will need a label. It's recommended the element takes the `group` role and is associated with the `<caption>` for the labeling. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe "labeling" is the American spelling, "labelling" is the British spelling. We should always use the British spellings.
src/components/infographics.md
Outdated
* Is scalable without degradation, facilitating zoom/magnification | ||
* Can be highly optimized, especially when using the [simple shapes and paths recommended](https://www.bbc.co.uk/gel/guidelines/how-to-design-infographics#infographic-visual-style-overview) | ||
* Can be maintained and updated easily, like any other markup | ||
* Unlike runtime-dependent `<canvas>` graphics, can contain accessibility information, and be downloaded as a self-sufficient asset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean "self-contained" asset?
|
||
Visual and figurative representations of information can help readers to quickly grasp the salient points. However, without careful and deliberate design they can be confusing, or even misleading. | ||
|
||
The visual design and labeling of infographics (or 'visualisations') is [covered comprehensively in GEL](https://www.bbc.co.uk/gel/guidelines/how-to-design-infographics). As a complement to the GEL documentation, here we shall explore two topics: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer British spelling "labelling".
* Unlike runtime-dependent `<canvas>` graphics, can contain accessibility information, and be downloaded as a self-sufficient asset | ||
|
||
Inline SVG (SVG embedded directly into the HTML) is the most malleable, since you can target individual SVG elements/artifacts with CSS and JavaScript. Embedding SVG also reduces http requests and, most importantly, _content reflows_[^2], by loading and rendering along with the surrounding content. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer British spelling "artefacts".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are honest differences of opinions about whether image assets in general should be served via separate HTTP requests, because those requests can be given different caching policies than the text, for example an image could have a 10-year publicly cachable policy based on the filename, whereas a personalised page migh need a no-cache policy. So this argument is nuanced and highly dependent on workflow, content and traffic patterns. I'd recommend sidestepping that debate by not raising the "reduces http requests" angle; The stronger usability argument is that it avoids reflows. In addition, even if a team has strictly separate workflows which demand that designers create images and coders create pages, it's fair to say that data visualisations are a special exception because they are not purely images (they are also data) so they belong with the text.
src/components/infographics.md
Outdated
|
||
### Labels and descriptions | ||
|
||
_Every_ infographic should be provided with alternative text (an accessible label) to describe it to the visually impaired. In addition, _some_ infographics should be accompanied by a caption. The role of the alternative text and the role of the caption are distinct. The alternative text _describes_, and the caption _explains_: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can include a ref or link to an "alternative text" guideline here?
src/components/data-tables.md
Outdated
|
||
### Column headers | ||
|
||
Any `<table>` that does not have column or row headers (`<th>` elements) cannot be considered a true data table. In fact, some screen readers that encounter a table without headers will determine it as a 'layout table' and communicate it quite differently[^1]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads better? "will treat it as a 'layout table'"
src/components/data-tables.md
Outdated
Note that the `<table>` is now divided into a head (`<thead>`) and body (`<tbody>`). This does not have any impact on the behavior of the table and its headers. But, as we enhance the table, these elements will come in useful for styling and scripting. | ||
|
||
::: alert Empty table headers | ||
In the last example, the first column header is effectively a header for the _row_ headers. Some deem this unnecessary and make the cell empty. This is not recommended, since it can produce unexpected behavior[^2]. It is also better to be explicit whenever there is the opportunity. In the [**Reference implementation**](#reference-implementation) the row headers are each premier league football teams. This could probably be concluded through deduction (and the help of surrounding information), but an explicit _"Team"_ column header removes all doubt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean the Premier League, it should be capitalised?
src/components/data-tables.md
Outdated
|
||
### Captions | ||
|
||
So far, we have labeled the rows and columns, but not the table itself. You may be inclined to introduce a table with a heading element, such as an `<h2>`. This would certainly aid users browsing the page visually. However, a heading would not be directly _associated_ with the table, meaning a screen reader user navigating directly to the table (using a shortcut like <kbd>T</kbd> in NVDA) would not hear a label announced upon arrival. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer British spelling "labelled".
## Recommended layout | ||
|
||
Importantly, the grid structure of data tables must remain intact no matter the available space. That is, elements must not wrap or otherwise change position since they will become labeled incorrectly. For tables with many columns this may precipitate horizontal scrolling. It is recommended a container element with `overflow-x: auto` is used to contain the horizontal scroll behavior. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads better? "this may result in horizontal scrolling"
src/components/data-tables.md
Outdated
``` | ||
|
||
::: alert Conditional interactivity | ||
It is not recommended the table container is made focusable by default, since tables that do not trigger a scrollbar would result in an interactive container that _doesn't do anything_. In the [**Reference implementation**](#reference-implementation) `tabindex="0"` is removed via `ResizeObserver` where the container is not overflowing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads better? "where the container does not overflow"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/bbc/gef-docs/blob/55a8738251c35a5d7d3a549454bda3775f47d4c5/docs/components/demos/data-table/index.html#L43-L51 I think your fingers might need oiling, @Heydon. 😄
Other than that, all's functioning well my side. It's awesome.
@micmath @simonsinclair Haha, okay. Thanks, both — I've fixed these. |
@Heydon just waiting for you to resolve the remaining change requests (happy to debate any of them you think I might have got wrong!) Also, it seems like the sticky table-of-contents was disabled in this PR? We tried to replicate the UX of the TOC seen here: https://www.bbc.co.uk/gel/guidelines/how-to-design-forms . Was the sticky behaviour removed intentionally? Can we get it back? |
@micmath Sorry, Michael. Which change requests are you referring to? I did the ones noted above (or did they not appear?). The sticky thing was removed as it was affecting the data tables, if you recall. However, I can reinstate that by using a different classname than |
@Heydon my github view of the list above shows the following are currently unchanged from the review:
it might be my own reading-fail but I'd be grateful if you could reconfirm you've updated those. Re the sticky TOC, my understanding was that you were planning to reimplement that in pure CSS, and thus wouldn't need the JS to make it work. As the PR currently stands it just doesn't work fullstop(?), so I suppose I'm asking about tha: Is it the case that the DataTables example cannot work on a page with a sticky TOC? If so I'd like to find a solution that doesn't require disabling it if possible? But I defer to you as you know more about the issues. |
@micmath Your suggestions were spot on and I changed the code but it may not have pushed. Just given my laptop to a conf for my talk in a bot, so this may have to be tomorrow now, but I will check this and make sure the sticky stuff is reinstated without breaking my tables. |
Awesome stuff, sir. 💃 |
@micmath I believe this is okay now. Sticky menu reinstated, with sticky tables working separately. Also, went over the other bits. I missed one in the infographics file which I've now caught. Plus: alt text reference, and rejigged footnotes. |
I pulled the latest changes to review, but there were a couple of issues that jumped out:
I assume you just dropped a copy-past somewhere for the introduction. I'll investigate the weird grey line, but possibly you are more familiar with the CSS and can spot the problem faster than me? |
@micmath You are quite right. I'll write that introduction now. |
The plot thickens. The stray line seems to be related to the css for the table. Can you hunt that bug? |
@micmath Just pushed the intro. |
@micmath Fixed and pushed. Basically, the stickiness results in no bottom border for the |
Thank you! |
These are now live, |
Includes both (interrelated) components, and their write-ups.