fix(table): targeted control-row sort button margining#7244
fix(table): targeted control-row sort button margining#7244mattnolting wants to merge 7 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-7244.surge.sh A11y report: https://patternfly-pr-7244-a11y.surge.sh |
mattnolting
force-pushed
the
fix-table-thead-cells
branch
from
fb0eb0c to
708c656
Compare
| padding-inline-end: var(--#{$table}--cell--first-last-child--PaddingInline); | ||
|
|
||
| .#{$table}__button { | ||
| margin-inline-end: calc((var(--#{$table}--cell--first-last-child--PaddingInline) - var(--#{$table}--cell--PaddingInlineend)) * -1); |
There was a problem hiding this comment.
| margin-inline-end: calc((var(--#{$table}--cell--first-last-child--PaddingInline) - var(--#{$table}--cell--PaddingInlineend)) * -1); | |
| margin-inline-end: calc((var(--#{$table}--cell--first-last-child--PaddingInline) - var(--#{$table}--cell--PaddingInlineEnd)) * -1); |
|
@mattnolting I noticed that the table
|
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
Left some comments. The line-height change could have some larger repercussions since anything in a cell that inherits line-height will no longer be relative to the text size of whatever is in the cell.
Also it may be related to @sg00dwin's comment, but the first "Source" table header in this demo is no longer left-aligned https://patternfly-pr-7244.surge.sh/components/table/html/nested-column-headers-example/
Also can you show how the original misalignment bug in #7220 applies to all tables, not just nested tables? Either a codepen/codesansbox or steps to reproduce like the steps in #7220.
| --#{$table}--cell--hidden-visible--Display: table-cell; | ||
|
|
||
| // * Table toggle | ||
| --#{$table}__toggle--PaddingBlockStart: var(--pf-t--global--spacer--sm); |
There was a problem hiding this comment.
Not sure if you meant to remove this? It's breaking to remove it, and it looks like it's still used here
|
|
||
| // * Table toggle | ||
| --#{$table}__toggle--PaddingBlockStart: var(--pf-t--global--spacer--sm); | ||
| --#{$table}__toggle--PaddingBlockEnd: var(--pf-t--global--spacer--sm); |
There was a problem hiding this comment.
Similar issue with this one - it's still used here
And then redefined here
| // TODO: update grouping comments to // * Table {element} | ||
|
|
||
| @include pf-root($table) { | ||
| --#{$table}--LineHeight: calc(var(--pf-t--global--font--line-height--body) * var(--pf-t--global--font--size--body--default)); |
There was a problem hiding this comment.
Since this is applied to cells, any text in a cell that 1) is not font--size--body--default and 2) inherits line-height will have the wrong line-height. Our empty state demo shows this - https://patternfly-pr-7244.surge.sh/components/table/html-demos/empty-state/
empty-state__body font size is 16px and the line-height is 21px now (14px * 1.5) instead of 24px.
5 participants
closes #7220
TLDR: Issue is being caused by table sort vertical alignment (should be
vertical-align: top) in tbody and negative margining.Backstop report
Because tables may contain elements with varying
line-height/font-size, allth/tdcells, whether intheadortbodyrequire a consistent and explicitline-heightto properly align vertically.