-
Notifications
You must be signed in to change notification settings - Fork 48
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 bordered badge #100
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a new bordered badge feature for the Flowbite Angular library. The implementation adds a Changes
Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (5)
🚧 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: 3
🧹 Nitpick comments (6)
apps/docs/docs/components/badge/_bordered.component.ts (1)
1-13
: Add unit tests for the bordered badge componentThe PR description indicates that tests are missing. Consider adding unit tests to verify:
- Component creation
- Host class application
- Template rendering with different badge variants
Would you like me to help generate the unit test suite for this component?
libs/flowbite-angular/badge/badge.theme.ts (1)
67-79
: Consider dark mode border color consistencyThe border colors in dark mode might need adjustment. Currently, some components use darker borders in dark mode (e.g., primary uses
border-primary-800
), while others use lighter borders (e.g., blue usesborder-blue-800
). This could lead to inconsistent visual hierarchy.Consider standardizing the dark mode border colors across all variants. For example:
-'bg-blue-100 dark:bg-blue-200 text-blue-800 dark:text-blue-800 group-hover:bg-blue-200 dark:group-hover:bg-blue-300 border-blue-300 dark:border-blue-800' +'bg-blue-100 dark:bg-blue-200 text-blue-800 dark:text-blue-800 group-hover:bg-blue-200 dark:group-hover:bg-blue-300 border-blue-300 dark:border-blue-400'apps/docs/docs/components/badge/index.md (1)
29-39
: Add code examples to the documentationWhile the demo component is referenced correctly, the documentation would be more helpful with actual code examples showing how to use the bordered badge, similar to other sections.
Consider adding usage examples:
## Bordered badge Use the `[hasBorder]="true"` property to add a border to your badge: ```angular-html <flowbite-badge [hasBorder]="true" color="primary">Primary bordered badge</flowbite-badge> <flowbite-badge [hasBorder]="true" color="dark">Dark bordered badge</flowbite-badge></blockquote></details> <details> <summary>libs/flowbite-angular/badge/badge.component.ts (3)</summary><blockquote> `47-50`: **Consider documenting the default value in a code comment.** The provider registration is correct, but adding a comment explaining why false is chosen as the default would improve maintainability. ```diff { + // Default to false for backward compatibility with existing badges provide: FLOWBITE_BADGE_HAS_BORDER_DEFAULT_VALUE, useValue: false, },
96-99
: Enhance property documentation to match other properties.The documentation should include the @default annotation and a more detailed description, consistent with other properties in the file.
/** - * Set if the badge has border + * Controls whether the badge displays with a border. + * When true, applies a border style based on the current color theme. + * + * @default false */ public hasBorder = model(inject(FLOWBITE_BADGE_HAS_BORDER_DEFAULT_VALUE));
128-128
: Implementation looks good, but tests are needed.The theme class integration is correct, but test coverage should be added for the new hasBorder property and its integration with the theme service.
Would you like me to help create test cases for:
- Default value injection
- hasBorder property binding
- Theme class generation with hasBorder
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/docs/docs/components/badge/_bordered.component.html
(1 hunks)apps/docs/docs/components/badge/_bordered.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
(4 hunks)libs/flowbite-angular/badge/badge.theme.service.ts
(1 hunks)libs/flowbite-angular/badge/badge.theme.ts
(3 hunks)
🔇 Additional comments (6)
apps/docs/docs/components/badge/ng-doc.page.ts (1)
2-2
: LGTM!
The bordered badge demo is properly integrated into the documentation page configuration.
Also applies to: 23-23
libs/flowbite-angular/badge/badge.theme.ts (2)
36-36
: LGTM: Interface changes are well-defined
The hasBorder
property is correctly typed as keyof FlowbiteBoolean
in both interfaces, maintaining type safety and consistency with the existing pattern.
Also applies to: 50-50
81-83
: LGTM: Border configuration is simple and effective
The boolean toggle between border
and border-0
classes is clean and follows Tailwind conventions.
libs/flowbite-angular/badge/badge.theme.service.ts (1)
34-34
: LGTM: Theme service implementation is correct
The hasBorder
property is correctly integrated into the class merging logic, maintaining consistency with other theme properties.
libs/flowbite-angular/badge/badge.component.ts (2)
22-24
: LGTM! Token declaration follows best practices.
The injection token is well-named, properly typed, and consistent with the existing pattern.
Line range hint 1-137
: Verify theme configuration for bordered badge.
Let's ensure the theme configuration includes the necessary styles for bordered badges.
✅ Verification successful
Theme configuration for bordered badge is properly implemented
The badge theme configuration includes all necessary styles for bordered badges:
- Border styles are defined for all color variants
hasBorder
property is properly configured withenabled/disabled
states- Theme service correctly applies the border classes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if badge theme configuration includes border styles
# Search for border-related styles in badge theme files
rg -l "border" libs/flowbite-angular/badge/badge.theme.ts
# Check badge theme interface for border properties
ast-grep --pattern 'interface BadgeProperties {
$$$
hasBorder: $_
$$$
}'
Length of output: 861
Script:
#!/bin/bash
# Let's check the actual theme implementation for border styles
rg -A 5 "border" libs/flowbite-angular/badge/badge.theme.ts
# Also check the theme service implementation
rg -A 5 "hasBorder" libs/flowbite-angular/badge/badge.theme.service.ts
Length of output: 2150
🎉 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: #90
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation