RHBT-1 : Added style override for Pf6 Table component#20
RHBT-1 : Added style override for Pf6 Table component#20AdityaPatil22 wants to merge 19 commits intopreviewfrom
Conversation
Created style override for pf-6 table component
|
nicolethoen
left a comment
There was a problem hiding this comment.
A quick overview of the scss - the use of hardcoded hex values is going to mean that this theme will not work in dark theme.
Adding the class .pf-v6-theme-dark class to the html element will flip the theme and in the demo app, you can see the side effect of using the hex values to set colors. If you can use semantic v6 tokens as hex values instead, that'll make it dark theme compatible out of the box.
otherwise, you'll need to define hex color values for these v6 semantic tokens/css variables when the dark theme class is present.
Added variables to hex values
|
Hey @nicolethoen Thank you for reviewing the PR. @AdityaPatil22 has replaced the hardcoded hex values with CSS variables and updated the corresponding values in the dark theme as well. You can preview the changes here: https://deploy-preview-6--demo-rhds-theme-for-pf6.netlify.app/ |
nicolethoen
left a comment
There was a problem hiding this comment.
I am happy to discuss either of my questions more. Ultimately it's up to you - The more the questions are resolved, the more future proof you are, as PF continues rolling out updates or as the rest of your ecosystem might update to use PF or PF6.
dist/redhat-brand-theme.css
Outdated
| .pf-v6-c-modal-box { | ||
| --pf-v6-c-modal-box--BorderRadius: var(--rh-border-radius-sharp, 0px); | ||
| :root { | ||
| --rh-color-light-gray: #fafafa; |
There was a problem hiding this comment.
The color palette shipping with PF6 is the complete brand color palette. So I’m wondering if - even if they don’t 100% match the old colors, if it’s worth making sure all colors selected with this red hat brand theme are from PF6's shipped color palette? What do you think?
dist/redhat-brand-theme.css
Outdated
|
|
||
| .pf-v6-c-table { | ||
| /* Header styles */ | ||
| --pf-v6-c-table__thead--cell--FontWeight: var(--pf-v5-c-table__thead--cell--FontWeight, 700); |
There was a problem hiding this comment.
continuing to reference --pf-v5- css variables means you’ll need to continue shipping all of the PF5 css in addition to the PF6 CSS to get this theme to work.
Is that unavoidable & will you always have the PF5 CSS in your environment? If the rest of the environment continues forward with modernization and migrations, you’ll have to rewrite all this CSS if you ever want to stop shipping PF5 styles.
src/scss/table-override.scss
Outdated
| } | ||
|
|
||
| .pf-v6-c-table__expandable-row .pf-v6-c-table__expandable-row-content { | ||
| padding-inline: 0px; |
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
|
|
||
| /* Selectors for compound expandable table variants */ | ||
| .pf-v6-c-table__compound-expansion-toggle.pf-m-expanded { | ||
| border-left-width: var(--rh-length-4xs, 1px); |
There was a problem hiding this comment.
Use border tokens for borders for all these, like:
--rh-border-width-sm
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
| .pf-v6-c-table { | ||
| /* Header styles */ | ||
| --pf-v6-c-table__thead--cell--FontWeight: var(--rh-font-weight-heading-bold, 700); | ||
| --pf-v6-c-table__thead--cell--FontSize: var(--rh-font-size-code-sm, 14px); |
There was a problem hiding this comment.
I believe this should either be the heading font-size token or the regular text font-size token, but not the code font-size token.
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
| /* Body styles */ | ||
| --pf-v6-c-table__tbody--cell--PaddingBlockStart: var(--rh-space-xl, 24px); | ||
| --pf-v6-c-table__tbody--cell--PaddingBlockEnd: var(--rh-space-xl, 24px); | ||
| --pf-v6-c-table__tbody--cell--FontSize: var(--rh-font-size-code-md, 16px); |
There was a problem hiding this comment.
Regular text font-size token.
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
| .pf-v6-c-table__compound-expansion-toggle:hover { | ||
| // border-left: var(--rh-length-4xs, 1px) solid var(--rh-color-gray-300, #d2d2d2); | ||
| // border-right: var(--rh-length-4xs, 1px) solid var(--rh-color-gray-300, #d2d2d2); | ||
| border-left-width: var(--rh-length-4xs, 1px); |
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
| .pf-v6-c-table.pf-m-sticky-header>.pf-v6-c-table__thead, | ||
| .pf-v6-c-table .pf-v6-c-table__thead.pf-m-nested-column-header { | ||
| // border-bottom: var(--rh-border-width-sm, 1px) solid var(--rh-color-canvas-white, #ffffff); | ||
| border-bottom-width: var(--rh-length-4xs, 1px); |
There was a problem hiding this comment.
Added the changes
src/scss/table-override.scss
Outdated
|
|
||
| /* Selectors for clickable table variants */ | ||
| .pf-v6-c-table tr:where(.pf-v6-c-table__tr).pf-m-clickable:is(:hover, :focus) { | ||
| box-shadow: 0 -0.1875rem 0.25rem -0.125rem rgba(3, 3, 3, 0.08), 0 0.1875rem 0.25rem -0.125rem rgba(3, 3, 3, 0.08); |
There was a problem hiding this comment.
We have a box-shadow token, which may apply here.
--rh-box-shadow-sm
There was a problem hiding this comment.
Added the changes
Replaced rh tokens for some stylings
|
Hi @markcaron, |
There was a problem hiding this comment.
I'm not sure why it's happening, but there are some visual discrepanices between the demo deploy preview (DP) and the examples on PF6 tables:
Striped Expandable
DP is missing some padding on the left of the expandable content and it's not 100% width. This leads me to believe some of the padding overrides are off.
PF Demo (I added pf-m-striped so we can see the expandable rows better)
Nested sticky header
Seems to be an errant \ between the cells:

Compound Expandable
This might just be the HTML in the DP not matching the PF demos, but the "tabs" aren't functional, and don't open/close when clicked twice, so it's hard to tell if things match exactly. I realize this is beyond the CSS changes in this PR.
| --pf-v6-c-button--BorderRadius: var(--rh-border-radius-default, 3px); | ||
| //--pf-v6-c-button--m-control--BorderRadius: var(--rh-border-radius-sharp, 0px); | ||
| } No newline at end of file | ||
| } |
There was a problem hiding this comment.
Maybe this will be handled in a separate PR, but since we're in this button-override.scss file...
The tertiary button should have border-color: var(--rh-color-border-strong, #151515); for Red Hat brand theming.
Since we're using it in the demo deploy preview, I just noticed it.
There was a problem hiding this comment.
Hi @markcaron,
- We investigated and did some research for the Striped Expandable table variant and found that the missing padding and the 100% width issue originates from the PF-6 original implementation.
-
We have also removed the errant backslashes "" between the cells.
-
Additionally, we reviewed the Compound Expandable table variant and tested it in both the local environment and the demo project. The tabs are functioning as expected and are consistent with the original PF-6 table specifications. If possible, could you please provide us with additional steps that we can follow to reproduce the issue that you are facing.
Screen.Recording.2025-07-02.at.5.21.14.PM.mov
- As part of the pf-button component updates, we have added
border-color: var(--rh-color-border-strong, #151515);to the tertiary button styling.
No description provided.