feat(scroll-top): add support for icon customisation #96
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a new scroll-to-top feature to the Angular application by adding the Changes
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/flowbite-angular/scroll-top/scroll-top.component.ts (1)
111-114: Enhance property documentationThe icon property documentation could be more descriptive. Consider adding:
- Default value information
- Usage example with TemplateRef
- Type information
Example documentation:
/** * Set the scroll top icon * @description Allows customization of the scroll-to-top button icon using a TemplateRef * @default undefined - Falls back to chevron-up icon * @example * <ng-template #customIcon> * <your-custom-icon /> * </ng-template> * <flowbite-scroll-top [icon]="customIcon" /> */ public icon = model(inject(FLOWBITE_SCROLL_TOP_ICON_DEFAULT_VALUE));apps/docs/src/app/app.component.html (1)
116-116: Consider adding icon customization exampleWhile the basic integration is correct, this would be a good place to showcase the new icon customization feature for documentation purposes.
Example implementation:
- <flowbite-scroll-top /> + <ng-template #customScrollTopIcon> + <flowbite-icon + svgIcon="outline:arrow-up" + class="w-5 h-5" /> + </ng-template> + <flowbite-scroll-top [icon]="customScrollTopIcon" />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/docs/src/app/app.component.html(1 hunks)apps/docs/src/app/app.component.ts(2 hunks)libs/flowbite-angular/scroll-top/scroll-top.component.ts(5 hunks)
🔇 Additional comments (4)
libs/flowbite-angular/scroll-top/scroll-top.component.ts (3)
35-37: LGTM: Well-structured injection token implementation
The injection token implementation follows Angular's best practices and provides good flexibility with optional template references.
Also applies to: 52-55
69-77: LGTM: Clean template implementation with proper fallback
The template elegantly handles both custom icons and fallback to default icon using modern Angular syntax.
Line range hint 64-78: LGTM: Well-structured component with performance optimizations
The component implementation follows Angular best practices:
- Uses OnPush change detection
- Proper encapsulation
- Clean inheritance hierarchy
apps/docs/src/app/app.component.ts (1)
17-17: LGTM: Clean component integration
The ScrollTopComponent is properly imported and integrated into the AppComponent.
Also applies to: 48-48
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
|
Caution No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
libs/flowbite-angular/scroll-top/scroll-top.component.ts (2)
35-37: Consider adding JSDoc comment for the injection token.The injection token is well-implemented, but adding documentation would improve maintainability.
+/** + * Injection token for customizing the scroll-top icon template. + * When not provided, falls back to the default chevron-up icon. + */ export const FLOWBITE_SCROLL_TOP_ICON_DEFAULT_VALUE = new InjectionToken< TemplateRef<unknown> | undefined >('FLOWBITE_SCROLL_TOP_ICON_DEFAULT_VALUE');
111-116: Enhance icon property documentation.While the basic documentation is present, it would be helpful to add more details about usage and examples.
/** * Set the scroll top icon + * + * @description + * Accepts a template reference to customize the scroll-to-top icon. + * When not provided, a default chevron-up icon will be displayed. + * + * @example + * ```html + * <flowbite-scroll-top> + * <ng-template #customIcon> + * <your-custom-icon /> + * </ng-template> + * </flowbite-scroll-top> + * ``` * * @default undefined */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/flowbite-angular/scroll-top/scroll-top.component.ts(5 hunks)
🔇 Additional comments (3)
libs/flowbite-angular/scroll-top/scroll-top.component.ts (3)
14-15: LGTM! Import changes are appropriate.
The addition of NgTemplateOutlet and type imports are correctly implemented for the new icon customization feature.
52-55: LGTM! Provider configuration is correct.
The default value of undefined is appropriate, allowing the component to fall back to the default icon when no custom icon is provided.
Line range hint 135-141: Verify signal cleanup and icon registration.
Please ensure:
- The icon signal is properly cleaned up when the component is destroyed
- The icon registration in init() is only done once
✅ Verification successful
No cleanup needed for icon registration
The implementation is correct as is. Here's why:
- The icon registration is handled by the
IconRegistryservice which is provided at root level and manages its own cleanup inngOnDestroy. - The
init()method inScrollTopComponentonly registers the icon once during component initialization, and duplicate registrations are handled internally by the registry. - Individual components don't need to clean up registered icons as they are managed centrally by the
IconRegistryservice which properly cleans up when the application is destroyed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for signal cleanup in component
ast-grep --pattern 'class $_ implements OnDestroy {
$$$
ngOnDestroy() {
$$$
this.icon.destroy();
$$$
}
$$$
}'
# Check for duplicate icon registration prevention
ast-grep --pattern 'class $_ {
$$$
init() {
if (this.iconRegistry.has("flowbite-angular", "chevron-up")) {
return;
}
$$$
}
$$$
}'
Length of output: 289
Script:
#!/bin/bash
# Let's try a broader search for the component implementation
rg -l "ScrollTopComponent" --type ts
# Check for any icon registration patterns
rg "addRawSvgIconInNamepsace" --type ts -A 2 -B 2
# Check for OnDestroy implementation
rg "implements OnDestroy" --type ts -A 5 "scroll-top"
# Check for init method implementation
rg "init\(\)" --type ts -A 5 "scroll-top"
# Look for the component file content
fd "scroll-top.component.ts" -t f -x cat {}
Length of output: 9885
Script:
#!/bin/bash
# Let's check the BaseComponent implementation for cleanup patterns
rg "class BaseComponent" --type ts -A 10
# Check if there are any icon cleanup patterns in the codebase
rg "destroy.*icon" --type ts -A 2 -B 2
# Look for the icon registry implementation
fd "icon-registry.ts" -t f -x cat {}
Length of output: 8204
MGREMY
deleted the
feat(scroll-top)-add-support-for-icon-customisation
branch
|
🎉 This PR is included in version 1.2.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: #85
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes