feat(misc, TextContent): update core version & text content updates by kmcfaul · Pull Request #10378 · patternfly/patternfly-react

kmcfaul · 2024-05-16T17:22:11Z

What: Closes #10314, #10262, #10335

Updated core version to alpha.135
Updated generateTokens, removing old, unused files it was pulling in to map vars
Updated Text component suite to use default content classes (Add support for classes on TextContent children #10262)
Updated Wizard to not use danger token directly & structure update (Update wizard to remove reference to error color token #10335)
Added blurb to docs about Text changes
Removed dropdown styles usage from Menu
Updated Charts font weight token global_FontWeight_bold to global_font_weight_heading_bold
Updated Slider inset token c_slider__step_Left to c_slider__step_InsetInlineStart
Updated Td/Th inset tokens c_table__sticky_cell_Left and c_table__sticky_cell_Right to c_table__sticky_cell_InsetInlineStart and c_table__sticky_cell_InsetInlineEnd
Updated Toolbar chip group class (variant prop still uses chip-group, but maps to pf-m-label-group now)
Updated Wizard Footer to use ActionList (also for deprecated)
Updated global height breakpoint token names
Updated snapshots
Fixed integration tests
Updated Table integration demo token global_primary_color_100 to global_color_brand_default
Fixed Table transformers test referencing dropdown styles to menu styles
Fixed react-token test, updated old token to new token

kmcfaul · 2024-05-16T17:24:14Z

I'm not sure if it's expected that a bunch of charts colors changed in the snapshots @mcoker Thoughts?

patternfly-build · 2024-05-16T17:30:08Z

Preview: https://patternfly-react-pr-10378.surge.sh

A11y report: https://patternfly-react-pr-10378-a11y.surge.sh

mcoker

This looks great! A comment on the wizard footer action list. Looks like the structure between <WizardFooter> and <WizardFooterInternal> are different, and unless my eyes are playing tricks on me, I think they're both different from core. I'd expect that to be:

wizard-footer
  action-list
    action-list-group
      action-list-item // back
      action-list-item // next
    action-list-group
      action-list-item // cancel

I think that should future-proof from needing HTML changes if we add more destructive and/or non-destructive actions in the future. I can elaborate more on that if you'd like.

packages/react-core/src/components/Text/examples/Text.md

packages/react-core/src/components/Text/examples/TextContentWrapped.tsx

packages/react-core/src/components/Wizard/WizardFooter.tsx

mcoker

@kmcfaul left some feedback. I think it's fine as-is if we want to merge it and follow up.

Re: the charts colors, I didn't review them all but I believe that's to be expected. We can review in depth with design but that can happen after this merges IMO.

packages/react-core/src/demos/examples/Card/CardStatus.tsx

packages/react-table/src/components/Table/examples/TableMisc.tsx

packages/react-core/src/demos/examples/Card/CardStatus.tsx

packages/react-core/src/demos/examples/Card/CardAggregateStatus.tsx

packages/react-tokens/README.md

mcoker · 2024-05-17T23:55:40Z

...components/NotificationDrawer/__tests__/__snapshots__/NotificationDrawerHeader.test.tsx.snap

  >
    <h1
-      class="pf-v6-c-notification-drawer__header-title"
+      class="pf-v6-c-content--h1 pf-v6-c-notification-drawer__header-title"


Any idea why we used <Text> here? It would have previously just generated an unstyled <h1> (unless wrapped in <TextContent>, which it doesn't look like it is), but now it's going to get a bunch of styles from the text/content component which we don't want AFAIK.

patternfly-react/packages/react-core/src/components/NotificationDrawer/NotificationDrawerHeader.tsx

Lines 40 to 43 in 4b8789a

<div {...props} className={css(styles.notificationDrawerHeader, className)}>

<Text component={TextVariants.h1} className={css(styles.notificationDrawerHeaderTitle)}>

{title}

</Text>

Removed the Text wrapper for a plain h1

packages/react-core/src/components/Wizard/WizardToggle.tsx

kmcfaul · 2024-05-20T15:50:09Z

@tlabaj Noticed that we had one last usage of deprecated SelectOption in a deprecated Table editable cell demo. I updated it to use the new SelectOption, but it looks like that demo doesn't open the select menu. Do we want to open a follow up for this? Or are we planning on eventually removing deprecated Table in v6?

kmcfaul · 2024-05-20T19:04:27Z

@patternfly/design-reviewers @patternfly/core-in-react-reviewers This PR should include the core updates to the Select outlined in #10011. If you don't mind using the surge here to re-verify the Select we can get that issue closed here too.

Edit: The core PR is actually still up, disregard this note.

mcoker

Just one small change needed for the wizard toggle.

Also left some comments on card demo icons but those are all pre-existing and just nits, so no importance to get in with this PR IMO.

Also there is a line in the react-tokens readme we could probably clean up -

patternfly-react/packages/react-tokens/README.md

Line 25 in 90b90a0

    
           import global_BackgroundColor_100 from '@patternfly/react-tokens/dist/esm/global_-background-color_100';

^ references an old global var name and the filename in the import has hyphens that should be underscores. We could probably just copy/paste the import line from the block below it there:

import global_background_color_primary_default from '@patternfly/react-tokens/dist/esm/global_background_color_primary_default';

packages/react-core/src/components/Wizard/WizardToggle.tsx

packages/react-core/src/demos/examples/Card/CardAggregateStatus.tsx

mcoker · 2024-05-20T21:12:47Z

packages/react-core/src/demos/examples/Card/CardStatus.tsx

          <Flex spaceItems={{ default: 'spaceItemsSm' }}>
            <FlexItem>
-              <CheckCircleIcon color={global_success_color_100.var} />
+              <CheckCircleIcon color={global_color_status_success_default.var} />


any reason we wouldn't use <Icon/> for the icons in this file, too?

Nope I don't think so, I will update them here too. Any time I think we don't have to directly use tokens we probably should.

thatblindgeye · 2024-05-21T12:20:01Z

packages/react-core/src/components/Toolbar/ToolbarItem.tsx

            styles.toolbarItem,
-            variant && styles.modifiers[toCamel(variant) as 'pagination' | 'label' | 'chipGroup'],
+            variant && styles.modifiers[toCamel(variant) as 'pagination' | 'label'],
+            variant === ToolbarItemVariant['chip-group'] && styles.modifiers.labelGroup,


Could we instead update the chip-group variant to label-group for the ToolbarItemVariant enum and prop type?

Opened #10405 to track that change in a follow up, along with the rest of Toolbar (as there are several usages of "chip" in its language for prop/component names).

…adme example

thatblindgeye

Above comment isn't a blocker and can be tackled in a followup

tlabaj · 2024-05-21T15:34:11Z

@tlabaj Noticed that we had one last usage of deprecated SelectOption in a deprecated Table editable cell demo. I updated it to use the new SelectOption, but it looks like that demo doesn't open the select menu. Do we want to open a follow up for this? Or are we planning on eventually removing deprecated Table in v6?

@kmcfaul Lets open a follow up for this for now.

tlabaj

lgtm

patternfly-build · 2024-05-21T15:59:24Z

Your changes have been released in:

@patternfly/react-charts@8.0.0-alpha.21
@patternfly/react-code-editor@6.0.0-alpha.59
@patternfly/react-core@6.0.0-alpha.59
@patternfly/react-docs@7.0.0-alpha.62
@patternfly/react-drag-drop@6.0.0-alpha.40
@patternfly/react-icons@6.0.0-alpha.21
@patternfly/react-integration@6.0.0-alpha.30
demo-app-ts@5.1.1-alpha.58
@patternfly/react-styles@6.0.0-alpha.21
@patternfly/react-table@6.0.0-alpha.59
@patternfly/react-templates@6.0.0-alpha.9
@patternfly/react-tokens@6.0.0-alpha.21

Thanks for your contribution! 🎉

feat(misc, TextContent): update core version & text content updates

b022d7a

update text examples and wording

f2c0967

This was linked to issues May 16, 2024

Updates to update and build latest core #10314

Closed

Add support for classes on TextContent children #10262

Closed

Update wizard to remove reference to error color token #10335

Closed

kmcfaul added 2 commits May 16, 2024 13:50

revert drawer integration, update docs ver, remove dupe dependency

3412822

update docs tokens, fix docs build

4b8789a

mcoker reviewed May 16, 2024

View reviewed changes

packages/react-core/src/components/Text/examples/Text.md Outdated Show resolved Hide resolved

packages/react-core/src/components/Text/examples/TextContentWrapped.tsx Show resolved Hide resolved

packages/react-core/src/components/Wizard/WizardFooter.tsx Show resolved Hide resolved

kmcfaul added 2 commits May 16, 2024 15:18

fix Wizard structure, update Text desc

f209711

revert integration test update

4e8e343

mcoker approved these changes May 18, 2024

View reviewed changes

pr feedback

90b90a0

mcoker approved these changes May 20, 2024

View reviewed changes

This was referenced May 21, 2024

Wizard - update custom footer example to use ActionList #10399

Closed

Various - Provide warnings for markup changes patternfly/pf-codemods#643

Closed

thatblindgeye reviewed May 21, 2024

View reviewed changes

update WizardToggle class, card demo parity with core, react-token re…

e602202

…adme example

thatblindgeye approved these changes May 21, 2024

View reviewed changes

tlabaj approved these changes May 21, 2024

View reviewed changes

tlabaj merged commit 1f1b93c into patternfly:v6 May 21, 2024

	<div {...props} className={css(styles.notificationDrawerHeader, className)}>
	<Text component={TextVariants.h1} className={css(styles.notificationDrawerHeaderTitle)}>
	{title}
	</Text>

Conversation

kmcfaul commented May 16, 2024

Uh oh!

kmcfaul commented May 16, 2024

Uh oh!

patternfly-build commented May 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcoker May 17, 2024

Choose a reason for hiding this comment

Uh oh!

kmcfaul May 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kmcfaul commented May 20, 2024

Uh oh!

kmcfaul commented May 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcoker May 20, 2024

Choose a reason for hiding this comment

Uh oh!

kmcfaul May 21, 2024

Choose a reason for hiding this comment

Uh oh!

thatblindgeye May 21, 2024

Choose a reason for hiding this comment

Uh oh!

kmcfaul May 21, 2024

Choose a reason for hiding this comment

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj commented May 21, 2024

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patternfly-build commented May 16, 2024 •

edited

Loading

kmcfaul commented May 20, 2024 •

edited

Loading