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

TAI-1139 added table--heavy-header #394

Merged
merged 1 commit into from
Feb 10, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions assets/scss/base/_table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ table {
font-weight: 400;
}
}

.table__header--highlight {
border-bottom: em(2) solid $govuk-blue-colour;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the use of the name heavy, is it the border or colour?
So given this is on the th element and there are no other header selectors I would be tempted to use:
.table-header
or
.table__header
Then if there are different iterations in the future we can add modifiers then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the default th styling not in effect the table__header? I would have thought that this is a variant of the default th? .table__header--heavy?

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this is a broader question for designers what is the default border styling for a table?
The work on the th should be a reset in base. The component should have a standard border size.

.table__header {
border-bottom: 2px solid;
border-color: <default>
}

Then we have a tones file

.tone-border-govuk {
border-colour: $govukBlue;
}

Implementation

<th class="table__header tone-border-govuk">

But that doesn't help the current situation!

Is the default th styling not in effect the table__header? I would have thought that this is a variant of the default th? .table__header--heavy?

Potentially .table__header--highlight although this feels a little odd to me without a .table__header class.
As tables are a guaranteed structure in html you get the table styles for free when adding a table without a selector, apply .table__header and you get the custom styles, this would be my ideal.

Or following the same patten then:

.table--header-highlight {
   th {
      border-bottom: em(2) solid $govuk-blue-colour;
   }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the:

.table--header-highlight {
th {
border-bottom: em(2) solid $govuk-blue-colour;
}
}

but shouldn't we try to keep the class as close as possible to the component that is being affected i.e. the th?

So it would be .table__header--highlight


.table--heavy-footer{
border-bottom: 3px solid $grey-6;
}
Expand Down