Skip to content

Conversation

@kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Nov 13, 2025

What: Closes #12066

Currently I've added support for "plain" based on the issue's usage (adding it to the existing variant prop), but I'm wondering whether this may be better as a separate isPlain flag. Though the NotificationBadge props are not 1:1 with Button's props, so maybe adding it as part of the variant prop is still fine. LMK if anyone has thoughts or preferences about one method or another.

@patternfly-build
Copy link
Collaborator

patternfly-build commented Nov 13, 2025

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.

An isPlain flag might slightly make more sense? As a value "plain" is pretty different from the other variant values, though at the same time we'd most likely have the logic setup the way it is in the PR now (I don't think we'd want a plain NotificationBadge to have the "attention" icon for example). I think it's good as-is, though.

@mcoker
Copy link
Contributor

mcoker commented Dec 12, 2025

Did we ever get design approval? I see the question in #12069 but no response

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 the goal is just a fourth variant where it's always plain.

IMO an isPlain prop might imply you can still use read/unread/attention variants? We can't do it now, but could with a small change.

Screenshot 2025-12-11 at 7 55 29 PM

@nicolethoen nicolethoen requested review from a team, lboehling and phcox and removed request for a team December 17, 2025 16:38
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.

Bug - NotificationBadge - variant="plain" but is undocumented

5 participants