fix(table): compact table expanded state styling#1563
Conversation
|
Deploy preview for pf-next ready! Built with commit 416c4a2 |
| @@ -0,0 +1,87 @@ | |||
| {{#> table table--id="compact-expandable-table" table--grid="true" table--modifier="pf-m-compact pf-m-grid-md" table--expandable="true" table--attribute='aria-label="Compact expandable table example"'}} | |||
There was a problem hiding this comment.
I wonder if we should design if compact expandable tables need to have the border modifier (once we merge your PR that adds them).
This looks off, but it might just be me
There was a problem hiding this comment.
re: borders, I think so. Is what looks off the lack of a border? between the top row and expanded row?
There was a problem hiding this comment.
but I think its a question from design, do you have the design specs?
There was a problem hiding this comment.
No, @mceledonia just said that the left border should behave like the regular expanded table example, but I agree it does look weird. Also not sure about the shadow on the right of the expanded area. That shadow fits better when there is a top border.
FWIW, here's what the compact table looks like in a better example with collapsed/expanded rows.
| {{#> table-td table-td--check="true"}} | ||
| <input type="checkbox" name="check-all" aria-label="Select all rows"> | ||
| {{/table-td}} | ||
| {{#> table-th table-th--attribute='scope="col"' table-th--sortable="true" table-th--modifier="pf-m-width-30" table-th--selected="true" table-th--desc="true"}} |
There was a problem hiding this comment.
I dont know if the compound expansion table needs the sortable feature in its demo
mcoker
force-pushed
the
issue-1560
branch
2 times, most recently
from
425aef1 to
ee985d7
Compare
|
@mceledonia @christiemolloy updated the spacing and border between the expandable row and the expanded content. |
mcoker
force-pushed
the
issue-1560
branch
2 times, most recently
from
74fedf8 to
03d77a3
Compare
|
|
||
| .pf-c-table__expandable-row { | ||
| .pf-c-table__expandable-row-content { | ||
| --pf-c-table__expandable-row-content--PaddingTop: var(--pf-c-table--m-compact__expandable-row-content--PaddingTop); |
There was a problem hiding this comment.
The paddings on all expandable-row-contents should be 24px.
I had updated table-grid.scss to this, but for some reason the compact table is not assigning it to the only child example (second one).
tr > * { // Remove First child padding left &:first-child { --pf-c-table-cell--PaddingLeft: 0; } // Remove Last child padding right &:last-child { --pf-c-table-cell--PaddingRight: 0; } }
There was a problem hiding this comment.
ah! Thanks. Updated, let me know if that addresses it :)
| @@ -1,4 +1,4 @@ | |||
| {{#> table newcontext table--id=table--id table--attribute=(concat table-nested--attribute ' id="' table--id '" aria-label="Nested table"') table--modifier="pf-m-compact" table--grid="true"}} | |||
| {{#> table newcontext table--id=table--id table--attribute=(concat table-nested--attribute ' id="' table--id '" aria-label="Nested table"') table--modifier="pf-m-compact pf-m-no-border-rows" table--grid="true"}} | |||
There was a problem hiding this comment.
What do you think about updating the border modifier to .pf-m-borderless-rows , .pf-m-no-border-rows sounds a little odd to me
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
fixes #1560