Skip to content

Add Notification Badge Component and some small fixes#316

Merged
hpehl merged 3 commits intopatternfly-java:mainfrom
mskacelik:issue-71
Sep 10, 2025
Merged

Add Notification Badge Component and some small fixes#316
hpehl merged 3 commits intopatternfly-java:mainfrom
mskacelik:issue-71

Conversation

@mskacelik
Copy link
Contributor

@mskacelik mskacelik commented Sep 9, 2025

Closes #71

Some additional fixes:

  • With the new JBang implementation, there was a problem with its preview mode [x]
  • Also, with JBang, new lines were not properly formatted in the snippet codes.
  • java.util.Calendar (and its constants) is not compatible with j2cl [x]

Regarding the NotificationBadge component:

  • I wanted to implement the Expandable interface, but it seems to serve a different use case. Most importantly, it changes during "expansion" by adding the Classes.expanded, and we don't want that.
  • When thinking about how to implement the read/unread/attention states, I came upon that the button implementation uses normal non-parametrized methods (primary(), secondary(), danger(), ...) instead of something like buttonStyle(Enum buttonStyle), is it because primary and secondary are from interfaces so for the sake of consistency other styles are also implemented like that? Just to be sure I used both approaches (with one utilizing the other).

@hpehl hpehl self-requested a review September 10, 2025 15:46
Copy link
Contributor

@hpehl hpehl left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @mskacelik

Normally the Expandable is meant to be implemented by components that can be visually expanded and collapsed. The drawer component is such a component. Looking at the notification drawer it doesn't really expand or collapse visually. It's just the state that is either expanded or collapsed. So I agree it doesn't make sense to implement the Expandable interface.

Regarding the button component and the decision to use non-parametrized methods like primary(), secondary(), and danger()instead of something like buttonStyle(Enum buttonStyle): I often ask myself whether the user defines the attribute once when the component is constructed or whether he needs to change the attribute at runtime.

In case of the button style I don't think it changes after the button has been created. So for me the different methods make more sense. In case of the notification badge I think both variants make sense. The different-methods-variant make the API more expressive when creating the notification badge. The one-method-with-parameter-variant makes it flexible enough so that the visual appearance can change based on some computation:

NotificationBadge badge = notificationBadge();
NotificationBadgeVariant variant = computeVariant(...);
badge.variant(variant);

So all in all, the PR looks really good to me. Well done!

@hpehl
Copy link
Contributor

hpehl commented Sep 10, 2025

And thanks for the other fixes 👍

@hpehl hpehl merged commit 4763c25 into patternfly-java:main Sep 10, 2025
1 check passed
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.

Implement notification badge component

2 participants