feat(Title, Text): apply penta updates#9889
Conversation
|
Preview: https://patternfly-react-pr-9889.surge.sh A11y report: https://patternfly-react-pr-9889-a11y.surge.sh |
tlabaj
requested review from
a team,
mfrances17 and
nicolethoen
and removed request for
a team
| <li className={css(icon && styles.listItem)} {...props}> | ||
| {icon && <span className={css(styles.listItemIcon)}>{icon}</span>} | ||
| {children} | ||
| <span className={css('pf-v5-c-list__item')}>{children}</span> |
There was a problem hiding this comment.
Could we avoid hard coding the classname here? use the object from react-styles instead?
There was a problem hiding this comment.
There is no object entry for this class is the issue, because it's not applying any styles currently. I can remove it?
There was a problem hiding this comment.
In that case, can we do something like what Adam did on this line: https://github.com/patternfly/patternfly-react/blob/main/packages/react-core/src/components/DualListSelector/DualListSelectorPane.tsx#L155
it avoids us having hard coded versioned prefixes
There was a problem hiding this comment.
Gotcha updated! Also caught upon double checking core that the class only gets applied if an icon is present so updated the logic/snapshots too.
kmcfaul
force-pushed
the
title-list-text-penta
branch
from
1dcc565 to
523c660
Compare
|
Rebased with core and looks like that fixed the weird token differences I was noticing, I was either slightly behind locally or had a cached file somewhere interfering. |
srambach
left a comment
srambach
left a comment
There was a problem hiding this comment.
Things seem to look good - my only comment would be that it would be nice to have the Title examples in the same order as core has them.
👍
andrew-ronaldson
left a comment
andrew-ronaldson
left a comment
There was a problem hiding this comment.
Happy days. Thanks
7 participants
What: Closes #9681
Title:
React translates heading level to size modifier classes (
pf-m-2xl) instead of directly using header modifiers (pf-m-h1).Text:
React is pulling in the text decoration token that specifies underline incorrectly (The other minor difference is that the--pf-t--global--link--text-decorationis set tounderlinein react but was updated tononein core). This should be resolved automatically with a core update.pf-m-visitedclass alone does not apply the color change in the react example, the link must be clicked for the:visitedstate before the purple color is applied. The react example should probably say "Link example" instead of "Visited link example".Edit - updated core fixed the underline issue.
List:
Icon font size differs between core and react due to outdated tokens (react translates down to--pf-t--global--icon--size--lg: 22pxwhile core translates to--pf-t--global--icon--size--lg: var(--pf-t--global--icon--size--250)which is16px). This should be resolved automatically with a core update.Edit - was out of date with core locally and icons look okay now beyond the slight difference that svgs cause.