-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat(Label): penta updates #10037
Conversation
Preview: https://patternfly-react-pr-10037.surge.sh A11y report: https://patternfly-react-pr-10037-a11y.surge.sh |
There was a problem hiding this 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.
There was a problem hiding this 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
Opened #10047 to follow up for status labels, in case it doesn't get in on this PR. |
Since these label types seem to be mutually exclusive with filled and outline. LMK if either should be a flag still instead. |
lgtm! |
There was a problem hiding this 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; |
There was a problem hiding this comment.
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.
Your changes have been released in:
Thanks for your contribution! 🎉 |
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.