[k82] Add table styles #34

travis-wb · 2020-07-06T20:42:47Z

Summary

k82's staff-facing UI requires tables.
https://paper.dropbox.com/doc/Tables--A3H49z0w6_fBe746G5FDNyQ~Ag-cUlKW2nUBzFD9b9MgvLXm

Changes are documented in this peak-design-system PR:
WealthBar/peak-design-system#43

Scope

The scope of this addition is to cover the use case of k82: staff-facing desktop view. The above documentation contains a few such notes about k82 scope. Some aspects of the complete table design will be marked for completion ASAP after release of k82.

Testing

You should be able to mark up a table in Wealthbar and have these new styles work well.

text should be left-aligned, except:
- you should be able to achieve right-aligned text in and <th> or <td> by applying the style .right
font size should be 1rem except:
- <th> actually uses a smaller font size
- you should be able to give the entire table smaller text by adding the class text-small
font weight should be normal, except:
- <th> gets a bold text style
- you should be able to achieve a bold style anywhere in a table by using the class .bold
rows should have a defualt light grey rollover style, except:
- headers don't have a rollover style
- adding class '.link-row` to a row should give it an alternative background colour
- you should be able to create text that shows a green 'link' style by giving it the class .link-row-target

Screenshot of WIP table as it will be used in k82:

Additionally, this new table style should not affect any existing table styles in WB. A PR was recently merged to solve known cases where it would interfere, if you find more please let me know

Submitter Checklist

Demo new or changed functionality to stakeholders
Perform self-review (see reviewer checklist)
Annotate MR with comments for reviewer
Document any new patterns or features on peak-design-system (will to ASAP after release of this PR)
Assign a team member who should specifically review this code
Address reviewer feedback, if any, and assign back to reviewer

Reviewer Checklist:

Version bump in package.json
Visually review significant UI changes
Assign @pez-wb as reviewer if needed
Assign back to submitter with feedback

travis-wb · 2020-07-06T20:48:45Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@wealthbar/peak-style",
-  "version": "1.7.5",
+  "version": "2.0.0",


since this styles the <table> element, it can be considered a breaking change. I've recently done a pre-remptive fix to try to prevent any breaks in WB.

travis-wb · 2020-07-06T20:49:52Z

scss/base/tables.scss

+    &:hover { background-color: $neutral-50 }
+  }
+
+  th, td { padding: 0.5rem 0 0.5rem 0.5rem; }


In a table you can't just slap padding on a <tr> so here we are

geoff-wb · 2020-07-06T21:10:30Z

scss/base/tables.scss

+  // TABLE VARIANT - smaller text
+  // this class can be applied to the <table> to shrink all text in the table
+  &.text-small {
+    font-size: 14px;


pixels! Not our favourite unit - rems?

fixed, thanks.

pez-work · 2020-07-06T21:19:50Z

scss/base/tables.scss

+  thead {
+    font-weight: bold;
+    border-bottom: 1px solid $neutral-300;
+    font-size: 14px;


is it tho? 😛

scss/base/tables.scss

pez-work · 2020-07-06T21:31:47Z

scss/base/tables.scss

+  // intended for use on a row to create a 'total' or 'summary' row style with bold text
+  .bold {
+    font-weight: bold;
+  }


I think this is already covered by the .strong class (global) in typography.scss

Right you are, removed it here and updated documentation accordingly

pez-work · 2020-07-06T21:33:14Z

scss/base/tables.scss

+  // this class can be applied to any <th> or <td>
+  .right {
+    text-align: right;
+  }


I think this could be globalized in the typography styles. Thoughts on that? (.center already exists there)

Sounds gravy to me. I've made the update.
Note that .center is not documented anywhere on the typography page. I've not blazed that trail, but I've mentioned both in documentation for tables.

scss/base/tables.scss

pez-work · 2020-07-14T16:53:34Z

scss/base/tables.scss

+
+  thead, th {
+    font-weight: bold;
+    font-size: 0.875rem;


are table headers always going to be small ??

just saying this seems exactly like a thing a desinger is going to change in a comp.

it's in the design specs, I dunno what else to tell ya:

Fair enough. For my sanity can you check w/ Julian to make sure he wants ALL tables to have small headers (JIC)

This is now Julian-confirmed for sanity.

scss/base/tables.scss

pez-work · 2020-07-14T18:30:20Z

scss/base/tables.scss

+      // TEXT VARIANT - hyperlink
+      // applying .link-row-target to a string will give it a link-like appearance
+      // when the mouse is hovered anywhere over the containing row
+      .link-row-target {


I get what is happening here with the targeted cell but this creates a scenario where the linked row has no visible "link" highlight text by default.

IMO it makes more sense to highlight (and underline! – it is a link) all text in the row by default and create a variant for the scenario where we only want to target one cell for highlight. Thoughts?

I think it's easy to imagine a scenario in which a table has many coumns, and there's a need to be clear about where the link goes. For example on k82, the columns show date, client, bank number, and a recent comment. The link's target is specific to the bank numbers. There could be multiple rows for that client, but the two rows would not link to the same place, so it makes sense to emphasize the unique target of the link.

It is true that this makes it possible to mark up a table without a defualt emphasized string, but designers should understand the design pattern here and shouldn't ask us to do that. It's also possible to mark up a tab UI with tabs that contain no content, or mark up a page with a top-level h2 and then have subheadings as h1s. That doesn't mean design is going to ask us to do those things. There's no precedent that sets an expectation that wrong markup should be impossible.

but designers should understand the design pattern here and shouldn't ask us to do that.

LOL.

From an interaction perspective within Peak anything that is a link defaults to $primary and the text has SOME form of hover/focus activated state. Like literally everything has this — the original Peak team defined it as such.

My questions for Julian would be: why is this different? And, how does text read as a link? The hover state mirrors that of a "standard" table but with green as opposed to grey. If there was no hover state on standard tables there would be enough differentiation to read as a link, but because non-link tables have this feature it just reads (to me anyway) as a variant — cursor changes and too subtle and not enough.

If you want I can discuss this with Julian whaile you move on but I really think this is something that should be addressed.

I'm hearing two concerns:
The 'contrast' concern - that the hover style has insufficient contrast.
The 'link' concern - that the link style can be incomplete if not used correctly.

Regarding the contrast: well we could measure it and see if it's up to accessibility standards.

Regarding the link: Can we think of any scenarios where applying the link style to EVERY column is a good idea? Example: You come across a table with columns date, client, bank number, and comment. They all look the same. What will happen when you click? Are you gonna see details about the client? Are you gonna see all activity for the date? Are you going to read the full version of the trancated column? Or, is each column linked separately? Will you go somewhere different depending hwich column you click on? You can't answer this question, so that's not a good solution.

re: contrast: I'd say its more differentiation and readability as a link the contrast per se.
re:link: yes, want to avoid ambiguity of usage as much as possible.

I actually really like the example scenario you gave because it highlights an interesting point: Is applying the link style to EVERY column is a good idea?

In the case we have now we are doing exactly that — highlighting every column in a row and making the whole thing a click target. Where does it go? what AM I going to see? is it this? or that? When in fact it is really just one column or part of a column that is the "link".

Maybe this is being over designed and over built.

if a user sees a green text w/ underline are they going to assume the whole row is a link target by default? probably not unless we tell/infer that it is (which is what we are doing with this interaction). Looking at this:

If we remove the row highlight, would the user not infer that the Desjardin -> Motus is the link? and really when THAT text highlights my gut would say ~80% of user would click on THAT text. I think this is a worth while interaction to get right and I think you make really valid points on this. TBH we should probably take this back to Julian and discuss the interaction and inference on this with him IMHO.

Julian helped me measure the contrast: We aim to pass AA

pez-work · 2020-07-14T18:31:18Z

scss/base/tables.scss

+
+    td {
+      padding-top: 0.75rem;
+      padding-bottom: 0.75rem;


why is cell padding different on link rows?

Ultimately I could clarify with Julian if we need to know the exact rationale.

For what it's worth, the design around this has undergone a pretty rigorous design phase. It was designed for k82, then revisited for Peak which involed putting the full specs together and a review within the design department. Not to put too fine a point on it, but at this stage I'd like to gloss over this question.

Ok just seems odd to have a potentally different row in the mix…

pez-work

a few minor comments but nothing cray. Maybe just verify w/ Julian on some of the implementation details.

scss/base/tables.scss

scss/patterns/cards.scss

scss/patterns/banners.scss

pez-work · 2020-07-16T17:12:08Z

scss/base/tables.scss

+
+  thead, th {
+    font-weight: bold;
+    font-size: 0.875rem;


Fair enough. For my sanity can you check w/ Julian to make sure he wants ALL tables to have small headers (JIC)

pez-work

travis-wb requested review from geoff-wb and pez-work July 6, 2020 20:44

travis-wb commented Jul 6, 2020

View reviewed changes

geoff-wb reviewed Jul 6, 2020

View reviewed changes

travis-wb force-pushed the k82.table branch from 797d4d2 to 364b60b Compare July 6, 2020 21:15

travis-wb changed the base branch from master to fix.css-files-regenerated July 6, 2020 21:17

travis-wb changed the base branch from fix.css-files-regenerated to master July 6, 2020 21:18

Add table styles

c8d41ed

travis-wb force-pushed the k82.table branch from 364b60b to c8d41ed Compare July 6, 2020 21:19

pez-work reviewed Jul 6, 2020

View reviewed changes

scss/base/tables.scss Show resolved Hide resolved

pez-work reviewed Jul 6, 2020

View reviewed changes

travis-wb mentioned this pull request Jul 6, 2020

[k82] Document new table styles WealthBar/peak-design-system#43

Merged

pez-work reviewed Jul 7, 2020

View reviewed changes

scss/base/tables.scss Outdated Show resolved Hide resolved

pez-work reviewed Jul 7, 2020

View reviewed changes

scss/base/tables.scss Outdated Show resolved Hide resolved

pez-work reviewed Jul 7, 2020

View reviewed changes

scss/base/tables.scss Show resolved Hide resolved

Review Fixes

b2c6ab8

travis-wb force-pushed the k82.table branch from ff3c73d to b2c6ab8 Compare July 7, 2020 18:28

travis-wb requested a review from pez-work July 8, 2020 00:14

Add border/shadow to table instead of using card

f568670

travis-wb force-pushed the k82.table branch from 835f9e4 to f568670 Compare July 8, 2020 18:11

pez-work suggested changes Jul 8, 2020

View reviewed changes

scss/base/tables.scss Show resolved Hide resolved

scss/base/tables.scss Outdated Show resolved Hide resolved

scss/base/tables.scss Show resolved Hide resolved

travis-wb added 2 commits July 9, 2020 11:19

Improve linked row structure

417d263

Improve file structure

f3c56c3

travis-wb force-pushed the k82.table branch from 004eef7 to f3c56c3 Compare July 9, 2020 22:22

travis-wb requested review from geoff-wb and pez-work July 9, 2020 22:26

pez-work reviewed Jul 14, 2020

View reviewed changes

travis-wb requested a review from pez-work July 14, 2020 18:10

pez-work reviewed Jul 14, 2020

View reviewed changes

scss/base/tables.scss Outdated Show resolved Hide resolved

pez-work reviewed Jul 14, 2020

View reviewed changes

Commonize drop-shadow as a variable

9b567f0

travis-wb changed the base branch from master to fix.css-files-regenerated July 15, 2020 16:12

travis-wb changed the base branch from fix.css-files-regenerated to master July 15, 2020 16:12

travis-wb requested a review from pez-work July 15, 2020 16:14

travis-wb force-pushed the k82.table branch from 56beb3b to 9b567f0 Compare July 15, 2020 16:48

travis-wb and others added 2 commits July 16, 2020 09:22

improve linked row appearance and code structure

ee7141b

Merge branch 'master' into k82.table

a9b07be

pez-work reviewed Jul 16, 2020

View reviewed changes

definitely totally final last final tweaks.

f0be77e

travis-wb force-pushed the k82.table branch from 1a4eb90 to f0be77e Compare July 16, 2020 22:25

pez-work approved these changes Jul 16, 2020

View reviewed changes

travis-wb merged commit be92079 into master Jul 17, 2020

travis-wb deleted the k82.table branch July 17, 2020 16:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[k82] Add table styles #34

[k82] Add table styles #34

travis-wb commented Jul 6, 2020 •

edited

Loading

travis-wb Jul 6, 2020 •

edited

Loading

travis-wb Jul 6, 2020 •

edited

Loading

geoff-wb Jul 6, 2020

travis-wb Jul 6, 2020

pez-work Jul 6, 2020

travis-wb Jul 6, 2020

pez-work Jul 8, 2020

pez-work Jul 6, 2020

travis-wb Jul 7, 2020

pez-work Jul 6, 2020

travis-wb Jul 7, 2020 •

edited

Loading

pez-work Jul 14, 2020

travis-wb Jul 14, 2020

pez-work Jul 15, 2020

travis-wb Jul 15, 2020

pez-work Jul 16, 2020

travis-wb Jul 16, 2020

pez-work Jul 16, 2020

pez-work Jul 14, 2020

travis-wb Jul 14, 2020 •

edited

Loading

pez-work Jul 15, 2020

travis-wb Jul 15, 2020

pez-work Jul 15, 2020

travis-wb Jul 15, 2020 •

edited

Loading

pez-work Jul 14, 2020

travis-wb Jul 14, 2020

travis-wb Jul 14, 2020

pez-work Jul 15, 2020

pez-work left a comment

pez-work Jul 16, 2020

pez-work left a comment

[k82] Add table styles #34

[k82] Add table styles #34

Conversation

travis-wb commented Jul 6, 2020 • edited Loading

Summary

Scope

Testing

Submitter Checklist

Reviewer Checklist:

travis-wb Jul 6, 2020 • edited Loading

Choose a reason for hiding this comment

travis-wb Jul 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travis-wb Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travis-wb Jul 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travis-wb Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pez-work left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pez-work left a comment

Choose a reason for hiding this comment

travis-wb commented Jul 6, 2020 •

edited

Loading

travis-wb Jul 6, 2020 •

edited

Loading

travis-wb Jul 6, 2020 •

edited

Loading

travis-wb Jul 7, 2020 •

edited

Loading

travis-wb Jul 14, 2020 •

edited

Loading

travis-wb Jul 15, 2020 •

edited

Loading