Add Notification Badge Component and some small fixes#316
Add Notification Badge Component and some small fixes#316hpehl merged 3 commits intopatternfly-java:mainfrom
Conversation
hpehl
left a comment
There was a problem hiding this comment.
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!
|
And thanks for the other fixes 👍 |
Closes #71
Some additional fixes:
java.util.Calendar(and its constants) is not compatible withj2cl[x]Regarding the
NotificationBadgecomponent:Expandableinterface, but it seems to serve a different use case. Most importantly, it changes during "expansion" by adding theClasses.expanded, and we don't want that.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).