refactor(table): change the paddings in the expandable content #1598

christiemolloy · 2019-03-19T15:29:37Z

patternfly-build · 2019-03-19T15:34:57Z

Deploy preview for pf-next ready!

Built with commit befe394

https://deploy-preview-1598--pf-next.netlify.com

mcoker · 2019-03-22T21:22:41Z

src/patternfly/components/Table/table.scss

+  td {
+    &.pf-m-no-padding {
+      padding: 0;
+      padding-left: var(--pf-c-table__expandable-row--before--Width); // set padding-left to adjust for left border.


Can you write this as padding: 0 0 0 var(--pf-c-table__expandable-row--before--Width);?

mcoker · 2019-03-22T21:26:55Z

src/patternfly/components/Table/examples/table-compound-expansion-example.hbs

@@ -26,19 +26,19 @@
      {{#> table-th table-th--data-label="Repository Name"}}
        <a href="#">siemur/test-space</a>
      {{/table-th}}
-      {{#> table-td table-td--compound-expansion-toggle="true" table-td--modifier="pf-m-expanded" table-td--data-label="Branches"}}
+      {{#> table-td table-td--compound-expansion-toggle="true" table-td--modifier="pf-m-expanded pf-m-no-padding" table-td--data-label="Branches"}}


did you mean to add pf-m-no-padding to the compound expansion toggle cells? Doesn't look like they have any padding by default.

yes that was a mistake thanks for catching that!

mcoker · 2019-03-22T21:28:29Z

src/patternfly/components/Table/examples/table-compound-expansion-example.hbs

-      {{#> table-td table-td--attribute='colspan="7"'}}
-        {{#> table-nested table--id=(concat table--id '-nested-table-1')}}
+      {{#> table-td table-td--attribute='colspan="7"' table-td--modifier="pf-m-no-padding"}}
+        {{#> table-nested table--id=(concat table--id '-nested-table-1"')}}


need to remove the extra double quotes in all of these nested table table--id's. those were fixed in https://github.com/patternfly/patternfly-next/pull/1614/files

Suggested change

{{#> table-nested table--id=(concat table--id '-nested-table-1"')}}

{{#> table-nested table--id=(concat table--id '-nested-table-1')}}

mcoker · 2019-03-22T21:41:48Z

src/patternfly/components/Table/examples/table-expandable-example.hbs

-      {{#> table-td table-td--attribute=(concat 'colspan="7" id="' table--id '-content1"')}}
+      {{#> table-td}}{{/table-td}}
+      {{#> table-td}}{{/table-td}}
+      {{#> table-td table-td--attribute=(concat 'colspan="4 id="' table--id '-content1"')}}
        <div class="pf-c-table__expandable-row-content">


should we create an hbs file for this? we use it a lot.

yes created one

mcoker · 2019-03-22T21:44:26Z

src/patternfly/components/Table/examples/table-expandable-example.hbs

    {{#> table-tr}}
-      {{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node2 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}
+      {{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node1 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}


Suggested change

{{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node1 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}

{{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node2 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}

mcoker · 2019-03-25T19:08:59Z

Can you verify this is what it's supposed to look like on mobile?

christiemolloy · 2019-03-25T20:31:41Z

@mcoker checked designs, I think its meant to be 32px padding left/right for the expandable content. As before it was 32px padding-left/right for the main content, then 64px for the expandable content. But after the change to 16px padding-left/right, I assume it will still be double padding. @kybaker can you review this padding on desktop and mobile ?

christiemolloy · 2019-03-26T14:32:49Z

@mcoker updated spacing here after speaking with @kybaker

mcoker · 2019-03-26T15:03:48Z

src/patternfly/components/Table/docs/table-expandable.md

+| `.pf-c-table__expandable-row` | `<tr>` | Initiates an expandable row. |
+| `.pf-c-table__expandable-row-content` | `.pf-c-table__expandable-row` > `<td>` > `<div>` | Initiates an expandable row content wrapper. |
+| `.pf-m-expanded` | `.pf-c-table__toggle` > `.pf-c-button`, `.pf-c-table__expandable-row` | Modifies for expanded state. |
+| `.pf-m-no-padding` | `.pf-c-expandable-row` > `<td>` | Modifies the expandable row to have no padding.


mcoker

nice!

patternfly-build · 2019-04-08T17:51:19Z

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

refactor(table): change the paddings in the expandable content

a8e698f

christiemolloy requested a review from mcoker March 19, 2019 15:29

christiemolloy added 2 commits March 22, 2019 17:15

update

46fd3b2

fix mc

6554ae8

mcoker reviewed Mar 22, 2019

View reviewed changes

christiemolloy added 5 commits March 25, 2019 09:58

updates from cokers review

3357e1a

update a11y

6d13e33

fix a11y

75783fd

update check id

c2643aa

fix a11y

9684021

update sizing

950e6b1

christiemolloy added the breaking change 💥 label Mar 26, 2019

change spacing

b5f411c

mcoker reviewed Mar 26, 2019

View reviewed changes

christiemolloy added 2 commits March 27, 2019 17:01

update wording

911384f

Merge branch 'master' into iss1507

befe394

mcoker approved these changes Mar 27, 2019

View reviewed changes

mcoker merged commit 5422d89 into patternfly:master Mar 27, 2019

mcoker mentioned this pull request Apr 1, 2019

Add pf-m-no-padding prop to expandable table row patternfly/patternfly-react#1678

Closed

patternfly-build added the released label Apr 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(table): change the paddings in the expandable content #1598

refactor(table): change the paddings in the expandable content #1598

christiemolloy commented Mar 19, 2019

patternfly-build commented Mar 19, 2019 •

edited

mcoker Mar 22, 2019

mcoker Mar 22, 2019

christiemolloy Mar 25, 2019

mcoker Mar 22, 2019

mcoker Mar 22, 2019

christiemolloy Mar 25, 2019

mcoker Mar 22, 2019

mcoker commented Mar 25, 2019

christiemolloy commented Mar 25, 2019

christiemolloy commented Mar 26, 2019

mcoker Mar 26, 2019

mcoker left a comment

patternfly-build commented Apr 8, 2019

	{{#> table-nested table--id=(concat table--id '-nested-table-1"')}}
	{{#> table-nested table--id=(concat table--id '-nested-table-1')}}

	{{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node1 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}
	{{#> table-td table-td--toggle="true" table-td--button--attribute=(concat 'aria-labelledby="' table--id '-node2 expandable-toggle2" id="expandable-toggle2" aria-label="Details" aria-controls="' table--id '-content2"')}}{{/table-td}}

	\| `.pf-m-no-padding` \| `.pf-c-expandable-row` > `<td>` \| Modifies the expandable row to have no padding.
	\| `.pf-m-no-padding` \| `.pf-c-table__expandable-row` > `<td>` \| Modifies the expandable row to have no padding.

refactor(table): change the paddings in the expandable content #1598

refactor(table): change the paddings in the expandable content #1598

Conversation

christiemolloy commented Mar 19, 2019

patternfly-build commented Mar 19, 2019 • edited

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

mcoker commented Mar 25, 2019

christiemolloy commented Mar 25, 2019

christiemolloy commented Mar 26, 2019

Choose a reason for hiding this comment

mcoker left a comment

Choose a reason for hiding this comment

patternfly-build commented Apr 8, 2019

patternfly-build commented Mar 19, 2019 •

edited