-
Notifications
You must be signed in to change notification settings - Fork 45
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(badge): add dismissable badge #101
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a new dismissible badge feature for the Flowbite Angular library. The implementation adds functionality to create badges that can be dismissed by users, with customizable colors and a close button. The changes span multiple files across the documentation and library components, adding new methods, properties, and styling options to support dismissable badges. The feature allows developers to create interactive badges with the ability to remove them dynamically. Changes
Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (6)
apps/docs/docs/components/badge/_dismissable.component.html (1)
1-49
: Consider refactoring repetitive badge definitionsThe current implementation has significant code duplication. Consider using an array of badge configurations and ngFor to reduce repetition.
Here's a suggested approach:
// In component class badges = [ { color: 'default', label: 'Default' }, { color: 'primary', label: 'Primary' }, { color: 'blue', label: 'Blue' }, // ... other colors ];<flowbite-badge *ngFor="let badge of badges" [isDismissable]="true" [onDismiss]="onDismiss" [color]="badge.color"> {{ badge.label }} </flowbite-badge>libs/flowbite-angular/badge/badge.theme.service.ts (1)
44-44
: LGTM! Consider adding tests for the new closeButtonClass.The implementation correctly merges the base and color classes for the close button using twMerge, following the established pattern.
Consider adding unit tests to verify the closeButtonClass generation with different color combinations.
apps/docs/docs/components/badge/index.md (1)
76-91
: LGTM! Consider enhancing the documentation.The new dismissable badge section follows the established documentation pattern.
Consider adding:
- Usage examples with different color variants
- Event handling examples for the dismiss action
- Accessibility considerations for the dismiss button
libs/flowbite-angular/badge/badge.component.ts (3)
95-109
: Consider enhancing keyboard accessibility.While the dismiss button implementation is good, consider adding keyboard event handling for better accessibility.
<button type="button" [class]="contentClasses()!.closeButtonClass" aria-label="Close" - (click)="onDismissClick()"> + (click)="onDismissClick()" + (keydown.enter)="onDismissClick()" + (keydown.space)="onDismissClick()">
166-177
: Consider using a more specific type for onDismiss callback.The onDismiss callback type could be more specific to better document the expected behavior.
- public onDismiss = model(inject(FLOWBITE_BADGE_ON_DISMISS_DEFAULT_VALUE)); + public onDismiss = model<((event: { source: BadgeComponent }) => void) | undefined>( + inject(FLOWBITE_BADGE_ON_DISMISS_DEFAULT_VALUE) + );This would allow passing the component instance to the callback for more context.
Line range hint
1-209
: Add unit tests for the dismissable badge feature.The implementation looks solid, but test coverage is missing. Consider adding tests for:
- Dismissable behavior
- Callback execution
- Icon registration
- Accessibility features
Would you like me to help generate unit tests for this component?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/docs/docs/components/badge/_dismissable.component.html
(1 hunks)apps/docs/docs/components/badge/_dismissable.component.ts
(1 hunks)apps/docs/docs/components/badge/index.md
(1 hunks)apps/docs/docs/components/badge/ng-doc.page.ts
(2 hunks)libs/flowbite-angular/badge/badge.component.ts
(8 hunks)libs/flowbite-angular/badge/badge.theme.service.ts
(1 hunks)libs/flowbite-angular/badge/badge.theme.ts
(2 hunks)
🔇 Additional comments (4)
apps/docs/docs/components/badge/ng-doc.page.ts (1)
4-4
: LGTM!
The dismissable component is properly imported and added to the demos object, following the established pattern.
Also applies to: 29-29
libs/flowbite-angular/badge/badge.theme.ts (2)
57-60
: LGTM! Theme interface is well-structured.
The closeButton theme interface follows the established pattern and includes necessary properties for styling.
126-128
: LGTM! Class interface is properly typed.
The BadgeClass interface correctly extends FlowbiteClass and includes the new closeButtonClass property.
libs/flowbite-angular/badge/badge.component.ts (1)
6-8
: LGTM! New imports and injection tokens are well-structured.
The new imports and injection tokens are properly implemented, following the existing patterns in the codebase.
Also applies to: 19-20, 45-51
🎉 This PR is included in version 1.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Issue Number
Issue Number: #89
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Bug Fixes