chore(deps): Bump v6 branch to latest core alpha#9774
chore(deps): Bump v6 branch to latest core alpha#9774mfrances17 merged 12 commits intopatternfly:v6from
Conversation
tlabaj
requested review from
a team,
mfrances17,
nicolethoen and
srambach
and removed request for
a team
|
Preview: https://patternfly-react-pr-9774.surge.sh A11y report: https://patternfly-react-pr-9774-a11y.surge.sh |
mfrances17
left a comment
mfrances17
left a comment
There was a problem hiding this comment.
just some nits, copy/paste error in multiple places :)
|
it'd be good to double check that any global tokens in the core repo are getting translated appropriately into react-tokens, just like the other css variables. |
srambach
left a comment
srambach
left a comment
There was a problem hiding this comment.
Notification badges don't look quite right - there's a weird outline and the hover is wrong. But these are also slightly still in flight along with some button work from @mcoker
Page is getting a scrollbar that isn't there in core. That might take a little more investigation.
There was a problem hiding this comment.
This is really cool!
couple quick questions:
I see that we are defining the token for blue_100 for example as:
export const _pf_t_color_blue_100 = {
"name": "--pf-t--color--blue--100",
"value": "#d9e9ff",
"var": "var(--pf-t--color--blue--100)"
};
export default _pf_t_color_blue_100;
Will that eventually be renamed and somehow replace the coexisting global variable?:
export const global_palette_blue_100 = {
"name": "--pf-v5-global--palette--blue-100",
"value": "#bee1f4",
"var": "var(--pf-v5-global--palette--blue-100)"
};
export default global_palette_blue_100;
EDIT: this might be exactly what your TODO in the PR description is referring to - so never mind :)
Also, the individual token files look good, but in patternfly_variables.js I see quite a few tokens that have value: undefined such as:
"_pf_t_global_color_nonstatus_orangered_100": {
"name": "--pf-t--global--color--nonstatus--orangered--100",
"value": "undefined",
"values": [
"--pf-t--color--orangered--200",
"undefined"
]
},
Is that expected?
|
I'm still seeing a difference in notification badge. I know there's an upcoming sprint to clean up notification badge but I don't know if there's a problem with what's in this PR or if it will be addressed. The workspace styles to make the dark background can be removed for sure. Otherwise, it looks ok to me and I'm fine moving forward and circling back. |
@srambach This can be addressed by the issue for notification badge #9811 |
6 participants
What: Closes #9411
Additional issues: towards #9811, #9808, and #9809
backgroundColorprop since light background is now the default with no option to change it..pf-v5-c-notification-badge__countno longer has a style associated with it so I updated the code with hardcoded placeholder string.themeprop since it only supports the default theme.light,darkand `darker variant since the are no longer supported.isSelectedprop since it is no longer supported in PageScreenshots of tokens BEFORE:
Screenshots of tokens AFTER:
