8000 feat(Table): add padding except for clickable expandable rows by kmcfaul · Pull Request #6958 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

feat(Table): add padding except for clickable expandable rows#6958

Merged
mcoker merged 2 commits intopatternfly:v6from
kmcfaul:table-expandable-padding
Aug 20, 2024
Merged

feat(Table): add padding except for clickable expandable rows#6958
mcoker merged 2 commits intopatternfly:v6from
kmcfaul:table-expandable-padding

Conversation

@kmcfaul
Copy link
Contributor
@kmcfaul kmcfaul commented Aug 12, 2024

Closes #6782.

Adjusts padding so it applies padding-block-start to compound expandable rows (and still removes padding for all other types of expandable row).

@patternfly-build
Copy link
Collaborator
patternfly-build commented Aug 12, 2024

@kmcfaul
Copy link
Contributor Author
kmcfaul commented Aug 12, 2024

Visual regression ran fine locally, just minor icon pixel differences (I think because Chromium is a different installation - Intel chip vs. ARM chip) and one test where the screenshot was taken before the icons loaded.

@kmcfaul kmcfaul linked an issue Aug 12, 2024 that may be closed by this pull request
Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Looks good afaict.
I will note that there's a comment down on line 1056 saying

  // TODO: make this a class `compound expansion content`
  .#{$table}__control-row ~ .#{$table}__expandable-row.pf-m-expanded {

that might be good to do in the future to simplify these selectors?

Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

L🐳TM

@mcoker mcoker merged commit 31f67a0 into patternfly:v6 Aug 20, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.0.0-alpha.218 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Table: compound expandable content missing padding

4 participants

0