Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Label): penta updates #10037

Merged
merged 3 commits into from
Jan 31, 2024
Merged

feat(Label): penta updates #10037

merged 3 commits into from
Jan 31, 2024

Conversation

kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Jan 23, 2024

What: Closes #9981

Added 'orangered' color option & added examples.
Added isAddLabel flag, intending for label group's add button.
Added pf-m-clickable to non-overflow and non-add label buttons or links.
Added pf-m-no-padding to close buttons.
Updated filled, outline, and compact examples to use css for layout instead of manual whitespace.

@kmcfaul kmcfaul linked an issue Jan 23, 2024 that may be closed by this pull request
@kmcfaul kmcfaul requested a review from mcoker January 23, 2024 21:04
@patternfly-build
Copy link
Contributor

patternfly-build commented Jan 23, 2024

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.

LGTM!

Just a heads up, I have another PR up in core to add status variants, and adds a "filled" modifier to the default/filled labels. It also replaces the info-circle icons we're using as the example icon in labels with a cube icon since the info-ircle icon is used with info labels.

Copy link
Contributor

@thatblindgeye thatblindgeye 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. Had a couple comments below, but they aren't blocking to this PR.

  • @mcoker the editable label cursor style remains a pointer when the label is currently editable. I'd expect the cursor to be text when editing a label (this would be a core followup, unless the intent was to have the pointer cursor)
  • Can we open a followup issue to add status support/update non-status labels in example to use the icon Michael mentioned above

@kmcfaul
Copy link
Contributor Author

kmcfaul commented Jan 26, 2024

Opened #10047 to follow up for status labels, in case it doesn't get in on this PR.

@kmcfaul
Copy link
Contributor Author

kmcfaul commented Jan 26, 2024

  • Removed isOverflowLabel and isAddLabel
  • Added overflow and add to variants

Since these label types seem to be mutually exclusive with filled and outline. LMK if either should be a flag still instead.

@lboehling
Copy link
Collaborator

lgtm!

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -58,8 +58,6 @@ export interface LabelProps extends React.HTMLProps<HTMLSpanElement> {
closeBtnProps?: any;
/** Href for a label that is a link. If present, the label will change to an anchor element. This should not be passed in if the onClick prop is also passed in. */
href?: string;
/** Flag indicating if the label is an overflow label. */
isOverflowLabel?: boolean;
Copy link
Contributor

@tlabaj tlabaj Jan 31, 2024

Choose a reason for hiding this comment

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

Can you please open codemod issue for this removed prop.

@tlabaj tlabaj merged commit 88d24da into patternfly:v6 Jan 31, 2024
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@6.0.0-alpha.22
  • @patternfly/react-core@6.0.0-alpha.22
  • @patternfly/react-docs@7.0.0-alpha.23
  • @patternfly/react-drag-drop@6.0.0-alpha.3
  • demo-app-ts@5.1.1-alpha.21
  • @patternfly/react-table@6.0.0-alpha.22

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Label): Consume core updates for Penta
7 participants