Skip to content
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

Only make column headers bold, not row headers #483

Merged
merged 1 commit into from
Jul 3, 2017
Merged

Conversation

selfthinker
Copy link
Contributor

@selfthinker selfthinker commented Jun 2, 2017

What problem does the pull request solve?

Having (row) headers is very important for accessibility.
But all of our table headers are bold, and that often doesn't look good with long row headers.
People who understand table semantics would need to overwrite that.
And people who don't understand semantics might choose not to use them.

We often have badly formatted tables. One reason for that is how govspeak works (I will raise an issue for that later). I suspect that another reason is that when you format a table correctly, it often doesn't look good. (But I don't have any evidence to back that up.)

E.g. if you formatted the tables on our bank holiday page correctly, it would currently look like this:
bank-holidays
(Although that doesn't use Elements but the example would be the same with Elements.)
And ideally, those headers shouldn't be bold. Although, there should be a way to make them bold (class for cells or modifier class for tables?).

This changes that so that only ths within a thead are bold. That might be controversial (and also be a breaking change), so I'd like your opinion on this. The question is also, how many services are using row headers?
What do you think?

This is how the table on the bank holiday page looks now, i.e. how a table within Elements would look with this change if it had row headers:
bank-holidays-original

What type of change is it?

  • Breaking change (fix or feature that would cause existing functionality to change)
    Although, I'm not 100% sure if it's really a breaking change? If someone's row headers turn non-bold because of this, would that be "breaking" it?

Having row headers is very important for accessibility.
But all of our table headers are bold,
and that often doesn't look good with long row headers.
People who understand table semantics would need to overwrite that.
And people who don't understand semantics might choose not to use them.
@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-483 June 2, 2017 15:53 Inactive
@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-483 June 2, 2017 15:56 Inactive
@edwardhorsford
Copy link
Contributor

This feels like a good change. Having all row headers bold is distracting and likely not what we want in most cases. It makes sense that authors might avoid using them because of this styling. Non-bold feels like a sensible default.

@joelanman
Copy link
Contributor

Agree with non bold row headers

@selfthinker selfthinker changed the title [Discuss] Only make column headers bold, not row headers Only make column headers bold, not row headers Jun 9, 2017
@edwardhorsford
Copy link
Contributor

@selfthinker can you include a screenshot of what the proposed change looks like?

@selfthinker
Copy link
Contributor Author

@edwardhorsford, if you can point me to a table using row headers, I could. But there is none within Elements. Not sure if it makes sense looking for an example used by a service (because I'm sure they would overwrite it)?

@edwardhorsford
Copy link
Contributor

@selfthinker what about the one above? Just to give an example of what it would look like. (and good if we ever refer to this PR again).

@selfthinker
Copy link
Contributor Author

Which one above? The table on the bank holiday page? That one doesn't have any row headers, I only used it as an example how it would look like if it had row headers. If it had them and it was using Elements and this was merged, it would look exactly the same as it looks now.

@edwardhorsford
Copy link
Contributor

@selfthinker exactly - I'm suggesting including a screenshot of here for posterity. In the future it may not look the same - better to have it as part of the PR.

@selfthinker
Copy link
Contributor Author

@edwardhorsford, okay, I've updated the original description with an additional screenshot.

@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-483 June 14, 2017 14:04 Inactive
@gemmaleigh gemmaleigh self-assigned this Jun 16, 2017
@gemmaleigh gemmaleigh added this to the 3.1.1 milestone Jun 16, 2017
Copy link

@kr8n3r kr8n3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, designers agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants