feat(FwbListGroupItem): Add href and to support #382
Conversation
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes refactor the list group item component and its documentation to support direct link and router-link functionality through new props ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ListGroupItem
participant Router
participant Browser
User->>ListGroupItem: Clicks on item
alt Item has "to" prop
ListGroupItem->>Router: Navigate via router-link
else Item has "href" prop
ListGroupItem->>Browser: Navigate to external link
else Item has @click handler
ListGroupItem->>ListGroupItem: Execute click handler
else
ListGroupItem->>ListGroupItem: No action (regular item)
end
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/components/listGroup/examples/FwbListGroupExampleLink.vue (1)
7-9: Correct the target attribute value.The target attribute value should be
"_blank"(single underscore) instead of"__blank"(double underscore) to comply with HTML standards.- <fwb-list-group-item href="https://flowbite.com" target="__blank"> + <fwb-list-group-item href="https://flowbite.com" target="_blank">docs/components/list-group.md (1)
54-56: Fix inconsistency in router link example.There's an inconsistency between the documentation example and the actual component. The documentation shows
to="#"while the component example usesto="/some-route". Consider using a more realistic path that mirrors the component example.- <fwb-list-group-item to="#"> + <fwb-list-group-item to="/some-route">src/components/FwbListGroup/FwbListGroupItem.vue (1)
65-70: Consider more flexible approach for determining link attribute.Currently,
linkAttronly checks for "router-link" and "nuxt-link" tags. Consider a more flexible approach that could accommodate custom components that also use the "to" prop.-const linkAttr = computed(() => { - if (!isLink.value) return null; - return props.tag === "router-link" || props.tag === "nuxt-link" - ? "to" - : "href"; -}); +const linkAttr = computed(() => { + if (!isLink.value) return null; + // If using to prop and a component that likely uses to, use to attribute + if (props.to && (props.tag !== "a" && props.tag !== "li")) { + return "to"; + } + // Otherwise default to href for links + return "href"; +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/components/button-group.md(1 hunks)docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue(1 hunks)docs/components/dropdown.md(1 hunks)docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue(2 hunks)docs/components/list-group.md(2 hunks)docs/components/listGroup/examples/FwbListGroupExampleHover.vue(0 hunks)docs/components/listGroup/examples/FwbListGroupExampleLink.vue(1 hunks)src/components/FwbListGroup/FwbListGroupItem.vue(2 hunks)
💤 Files with no reviewable changes (1)
- docs/components/listGroup/examples/FwbListGroupExampleHover.vue
🔇 Additional comments (10)
docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue (1)
13-17: Implementation looks good - clean and simplified markup!The list group items now directly use the
hrefattribute instead of nested anchor tags, making the markup cleaner and more semantic. This takes advantage of the enhancedFwbListGroupItemcomponent that now supports link functionality.docs/components/button-group.md (1)
83-87: Documentation updated correctly to match implementationThe code examples in the documentation have been properly updated to use the direct
hrefattributes on list group items, matching the implementation changes. This ensures documentation and code remain in sync.docs/components/dropdown.md (1)
147-158: Great demonstration of the different use casesThis example effectively showcases the component's versatility by demonstrating:
- Router link navigation with
to="#"(line 147)- External links with
href="#"(lines 150, 153)- Click event handling with
@click="signOut"(line 156)The implementation is clean and provides good examples of all supported use cases.
docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue (2)
8-19: Good implementation showing all navigation patternsThe changes effectively demonstrate the three different navigation patterns:
- Router navigation with
to="my-dashboard"- External links with
href="#"- Click event handling with
@click="signOut"This showcases the flexibility of the enhanced
FwbListGroupItemcomponent.
27-27: Empty signOut implementation is appropriate for this exampleThe empty
signOutfunction serves as a placeholder for demonstration purposes. This is appropriate for an example component where the actual sign-out functionality would be implemented in a real application.docs/components/listGroup/examples/FwbListGroupExampleLink.vue (1)
1-14: Well-structured example demonstrating all link scenarios.The example effectively showcases the four main use cases of the enhanced component:
- An active link with href
- An external link with target attribute
- A router link with the to prop
- A regular non-link item without hover effect
This makes the component's new capabilities clear to users.
docs/components/list-group.md (1)
39-43: Clear and concise explanation of the new link functionality.The tip provides a good explanation of when to use each prop (
hreffor external links,tofor router links, andtargetfor setting the target attribute). The note about automatic hover effect is particularly helpful for users migrating from the previous version.src/components/FwbListGroup/FwbListGroupItem.vue (3)
2-7: Well-implemented dynamic component rendering.The dynamic component implementation elegantly handles multiple rendering scenarios (regular item, external link, router link) with a single template. The conditional attribute binding ensures that the appropriate link attribute (
hreforto) is used, and the target attribute is only applied when needed.
33-49: Good implementation of new props with appropriate defaults.The new props are well-defined with appropriate types and default values. The null defaults for
toandhrefallow for clear conditional logic based on their presence.
76-81: Great implementation of automatic hover detection.The automatic hover detection (based on link presence or click handler) is an elegant solution that simplifies the API by removing the need for an explicit hover prop. This improves the developer experience while maintaining backward compatibility.
Sqrcz
added
🔧 enhancement
|
Looks great. Very needed features. Thank you! There may be some linting issue on the first glance... I'll take a proper look and make thorough review early next week. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/components/listGroup/examples/FwbListGroupExampleLink.vue (2)
7-9: Fix the target attribute valueThe target attribute should be
"_blank"(single underscore) instead of"__blank"(double underscore). This is the standard HTML attribute value for opening links in a new tab.- <fwb-list-group-item href="https://flowbite.com" target="__blank"> + <fwb-list-group-item href="https://flowbite.com" target="_blank">
16-18: Fix linting error with semicolonThere's a linting error regarding an extra semicolon.
-import { FwbListGroup, FwbListGroupItem } from '../../../../src/index'; +import { FwbListGroup, FwbListGroupItem } from '../../../../src/index'🧰 Tools
🪛 ESLint
[error] 17-17: Extra semicolon.
(@stylistic/semi)
src/components/FwbListGroup/FwbListGroupItem.vue (1)
25-27: Fix import order and remove extra semicolonsThere are linting errors regarding the import order and unnecessary semicolons throughout the file.
-import { computed, resolveComponent, useAttrs, toRef } from 'vue'; +import { computed, resolveComponent, toRef, useAttrs } from 'vue' -import { useListGroupItemClasses } from './composables/useListGroupItemClasses'; +import { useListGroupItemClasses } from './composables/useListGroupItemClasses'Note: Similar semicolon issues exist throughout the file and should be addressed.
🧰 Tools
🪛 ESLint
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
[error] 27-27: Extra semicolon.
(@stylistic/semi)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/components/button-group.md(1 hunks)docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue(1 hunks)docs/components/dropdown.md(1 hunks)docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue(2 hunks)docs/components/list-group.md(2 hunks)docs/components/listGroup/examples/FwbListGroupExampleHover.vue(0 hunks)docs/components/listGroup/examples/FwbListGroupExampleLink.vue(1 hunks)src/components/FwbListGroup/FwbListGroupItem.vue(3 hunks)
💤 Files with no reviewable changes (1)
- docs/components/listGroup/examples/FwbListGroupExampleHover.vue
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue
- docs/components/button-group.md
- docs/components/dropdown.md
- docs/components/list-group.md
- docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue
🧰 Additional context used
🪛 ESLint
docs/components/listGroup/examples/FwbListGroupExampleLink.vue
[error] 17-17: Extra semicolon.
(@stylistic/semi)
src/components/FwbListGroup/FwbListGroupItem.vue
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
[error] 27-27: Extra semicolon.
(@stylistic/semi)
[error] 29-29: Extra semicolon.
(@stylistic/semi)
[error] 55-55: Extra semicolon.
(@stylistic/semi)
[error] 57-57: Extra semicolon.
(@stylistic/semi)
[error] 58-58: Extra semicolon.
(@stylistic/semi)
[error] 60-60: Must use .value to read or write the value wrapped by computed().
(vue/no-ref-as-operand)
[error] 61-61: Extra semicolon.
(@stylistic/semi)
[error] 63-63: Extra semicolon.
(@stylistic/semi)
[error] 64-64: Extra semicolon.
(@stylistic/semi)
[error] 66-66: Extra semicolon.
(@stylistic/semi)
[error] 68-68: Extra semicolon.
(@stylistic/semi)
[error] 70-70: Extra semicolon.
(@stylistic/semi)
[error] 71-71: Extra semicolon.
(@stylistic/semi)
[error] 73-73: Extra semicolon.
(@stylistic/semi)
[error] 74-74: Must use .value to read or write the value wrapped by computed().
(vue/no-ref-as-operand)
[error] 76-76: Extra semicolon.
(@stylistic/semi)
[error] 77-77: Extra semicolon.
(@stylistic/semi)
[error] 78-78: Extra semicolon.
(@stylistic/semi)
[error] 79-79: Extra semicolon.
(@stylistic/semi)
[error] 81-81: Extra semicolon.
(@stylistic/semi)
[error] 82-82: Extra semicolon.
(@stylistic/semi)
[error] 83-83: Extra semicolon.
(@stylistic/semi)
[error] 85-85: Extra semicolon.
(@stylistic/semi)
🔇 Additional comments (4)
docs/components/listGroup/examples/FwbListGroupExampleLink.vue (1)
3-12: Well-structured example showcasing all link variationsThis example effectively demonstrates the different ways the list group item can be used with the new link capabilities: as an active link, external link, router link, and regular item. Good job providing a comprehensive example!
src/components/FwbListGroup/FwbListGroupItem.vue (3)
57-83: Excellent implementation of link supportThe refactoring of this component to support both standard HTML links and router links is well-implemented. The computed properties effectively determine the component's behavior based on the provided props. The automatic hover detection based on link status or click handlers is a particularly nice touch that will improve developer experience.
🧰 Tools
🪛 ESLint
[error] 57-57: Extra semicolon.
(@stylistic/semi)
[error] 58-58: Extra semicolon.
(@stylistic/semi)
[error] 60-60: Must use
.valueto read or write the value wrapped bycomputed().(vue/no-ref-as-operand)
[error] 61-61: Extra semicolon.
(@stylistic/semi)
[error] 63-63: Extra semicolon.
(@stylistic/semi)
[error] 64-64: Extra semicolon.
(@stylistic/semi)
[error] 66-66: Extra semicolon.
(@stylistic/semi)
[error] 68-68: Extra semicolon.
(@stylistic/semi)
[error] 70-70: Extra semicolon.
(@stylistic/semi)
[error] 71-71: Extra semicolon.
(@stylistic/semi)
[error] 73-73: Extra semicolon.
(@stylistic/semi)
[error] 74-74: Must use
.valueto read or write the value wrapped bycomputed().(vue/no-ref-as-operand)
[error] 76-76: Extra semicolon.
(@stylistic/semi)
[error] 77-77: Extra semicolon.
(@stylistic/semi)
[error] 78-78: Extra semicolon.
(@stylistic/semi)
[error] 79-79: Extra semicolon.
(@stylistic/semi)
[error] 81-81: Extra semicolon.
(@stylistic/semi)
[error] 82-82: Extra semicolon.
(@stylistic/semi)
[error] 83-83: Extra semicolon.
(@stylistic/semi)
2-7: Great use of dynamic component renderingThe implementation of the dynamic
<component>element with proper attribute binding is well done. This approach provides excellent flexibility for rendering different types of elements based on the props.Also applies to: 21-21
39-54: Well-structured new props with appropriate defaultsThe new props (
to,href,target,tag) are well-defined with appropriate types and default values. This provides a clear API for component users.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/components/listGroup/examples/FwbListGroupExampleLink.vue (1)
17-17: Remove extra semicolonAccording to the project's linting rules, there shouldn't be a semicolon at the end of this import statement.
-import { FwbListGroup, FwbListGroupItem } from '../../../../src/index'; +import { FwbListGroup, FwbListGroupItem } from '../../../../src/index'🧰 Tools
🪛 ESLint
[error] 17-17: Extra semicolon.
(@stylistic/semi)
src/components/FwbListGroup/FwbListGroupItem.vue (2)
25-85: Fix linting issues with semicolonsThere are multiple semicolons at the end of statements throughout the file that should be removed to comply with the project's linting rules.
Consider running a linter to automatically fix these issues, or manually remove the semicolons at the end of lines.
🧰 Tools
🪛 ESLint
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
[error] 27-27: Extra semicolon.
(@stylistic/semi)
[error] 29-29: Extra semicolon.
(@stylistic/semi)
[error] 55-55: Extra semicolon.
(@stylistic/semi)
[error] 57-57: Extra semicolon.
(@stylistic/semi)
[error] 58-58: Extra semicolon.
(@stylistic/semi)
[error] 61-61: Extra semicolon.
(@stylistic/semi)
[error] 63-63: Extra semicolon.
(@stylistic/semi)
[error] 64-64: Extra semicolon.
(@stylistic/semi)
[error] 66-66: Extra semicolon.
(@stylistic/semi)
[error] 68-68: Extra semicolon.
(@stylistic/semi)
[error] 70-70: Extra semicolon.
(@stylistic/semi)
[error] 71-71: Extra semicolon.
(@stylistic/semi)
[error] 73-73: Extra semicolon.
(@stylistic/semi)
[error] 76-76: Extra semicolon.
(@stylistic/semi)
[error] 77-77: Extra semicolon.
(@stylistic/semi)
[error] 78-78: Extra semicolon.
(@stylistic/semi)
[error] 79-79: Extra semicolon.
(@stylistic/semi)
[error] 81-81: Extra semicolon.
(@stylistic/semi)
[error] 82-82: Extra semicolon.
(@stylistic/semi)
[error] 83-83: Extra semicolon.
(@stylistic/semi)
[error] 85-85: Extra semicolon.
(@stylistic/semi)
25-25: Fix import member orderingAccording to the linting rules, the import members should be alphabetically sorted.
-import { computed, resolveComponent, useAttrs, toRef } from 'vue'; +import { computed, resolveComponent, toRef, useAttrs } from 'vue'🧰 Tools
🪛 ESLint
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/components/button-group.md(1 hunks)docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue(1 hunks)docs/components/dropdown.md(1 hunks)docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue(2 hunks)docs/components/list-group.md(2 hunks)docs/components/listGroup/examples/FwbListGroupExampleHover.vue(0 hunks)docs/components/listGroup/examples/FwbListGroupExampleLink.vue(1 hunks)src/components/FwbListGroup/FwbListGroupItem.vue(3 hunks)
💤 Files with no reviewable changes (1)
- docs/components/listGroup/examples/FwbListGroupExampleHover.vue
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/components/dropdown.md
- docs/components/buttonGroup/examples/FwbButtonGroupExampleDropdown.vue
- docs/components/button-group.md
- docs/components/dropdown/examples/FwbDropdownExampleListGroup.vue
- docs/components/list-group.md
🧰 Additional context used
🪛 ESLint
docs/components/listGroup/examples/FwbListGroupExampleLink.vue
[error] 17-17: Extra semicolon.
(@stylistic/semi)
src/components/FwbListGroup/FwbListGroupItem.vue
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
[error] 27-27: Extra semicolon.
(@stylistic/semi)
[error] 29-29: Extra semicolon.
(@stylistic/semi)
[error] 55-55: Extra semicolon.
(@stylistic/semi)
[error] 57-57: Extra semicolon.
(@stylistic/semi)
[error] 58-58: Extra semicolon.
(@stylistic/semi)
[error] 61-61: Extra semicolon.
(@stylistic/semi)
[error] 63-63: Extra semicolon.
(@stylistic/semi)
[error] 64-64: Extra semicolon.
(@stylistic/semi)
[error] 66-66: Extra semicolon.
(@stylistic/semi)
[error] 68-68: Extra semicolon.
(@stylistic/semi)
[error] 70-70: Extra semicolon.
(@stylistic/semi)
[error] 71-71: Extra semicolon.
(@stylistic/semi)
[error] 73-73: Extra semicolon.
(@stylistic/semi)
[error] 76-76: Extra semicolon.
(@stylistic/semi)
[error] 77-77: Extra semicolon.
(@stylistic/semi)
[error] 78-78: Extra semicolon.
(@stylistic/semi)
[error] 79-79: Extra semicolon.
(@stylistic/semi)
[error] 81-81: Extra semicolon.
(@stylistic/semi)
[error] 82-82: Extra semicolon.
(@stylistic/semi)
[error] 83-83: Extra semicolon.
(@stylistic/semi)
[error] 85-85: Extra semicolon.
(@stylistic/semi)
🔇 Additional comments (5)
docs/components/listGroup/examples/FwbListGroupExampleLink.vue (1)
1-14: Clear and comprehensive examples of the new link functionality!This example file effectively demonstrates all the key use cases for the enhanced list group item component:
- Using
hreffor regular links (with active state)- Using
hrefwithtargetfor external links- Using
tofor router links- Regular item without link behavior
This matches perfectly with the PR objectives of adding link support.
src/components/FwbListGroup/FwbListGroupItem.vue (4)
2-7: Great use of dynamic component rendering!The template now intelligently adapts based on the props provided:
- Uses dynamic
:isbinding to render the appropriate element type- Dynamically assigns either
hrefortobased on the link type- Conditionally applies the target attribute only when needed for external links
This implementation is both flexible and semantically correct.
Also applies to: 21-21
25-55: Well-structured props for enhanced link functionalityThe addition of
to,href,target, andtagprops provides a comprehensive API for link handling:
hreffor standard linkstofor router linkstargetfor controlling how links opentagfor custom element renderingThese additions fulfill the PR objective of supporting both external and router-based links.
🧰 Tools
🪛 ESLint
[error] 25-25: Member 'toRef' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 25-25: Extra semicolon.
(@stylistic/semi)
[error] 27-27: Extra semicolon.
(@stylistic/semi)
[error] 29-29: Extra semicolon.
(@stylistic/semi)
[error] 55-55: Extra semicolon.
(@stylistic/semi)
57-79: Clean implementation of dynamic component and attribute logicThe computed properties provide clear logic for determining:
- Whether the item should be a link
- What component type to use (li, a, router-link, nuxt-link)
- Which attribute to use for the link target (href or to)
- When to show the target attribute
This approach makes the component highly flexible while maintaining clear, readable code.
🧰 Tools
🪛 ESLint
[error] 57-57: Extra semicolon.
(@stylistic/semi)
[error] 58-58: Extra semicolon.
(@stylistic/semi)
[error] 61-61: Extra semicolon.
(@stylistic/semi)
[error] 63-63: Extra semicolon.
(@stylistic/semi)
[error] 64-64: Extra semicolon.
(@stylistic/semi)
[error] 66-66: Extra semicolon.
(@stylistic/semi)
[error] 68-68: Extra semicolon.
(@stylistic/semi)
[error] 70-70: Extra semicolon.
(@stylistic/semi)
[error] 71-71: Extra semicolon.
(@stylistic/semi)
[error] 73-73: Extra semicolon.
(@stylistic/semi)
[error] 76-76: Extra semicolon.
(@stylistic/semi)
[error] 77-77: Extra semicolon.
(@stylistic/semi)
[error] 78-78: Extra semicolon.
(@stylistic/semi)
[error] 79-79: Extra semicolon.
(@stylistic/semi)
81-85: Elegant automatic hover detectionThis implementation elegantly solves the hover state problem:
- Automatic detection when a link attribute or click handler is present
- No need for an explicit hover prop
- Maintains reactivity by using
toRefandcomputedproperlyThis delivers on the PR objective of "automatic hover effect detection when a click event handler is present."
🧰 Tools
🪛 ESLint
[error] 81-81: Extra semicolon.
(@stylistic/semi)
[error] 82-82: Extra semicolon.
(@stylistic/semi)
[error] 83-83: Extra semicolon.
(@stylistic/semi)
[error] 85-85: Extra semicolon.
(@stylistic/semi)
|
Anytime! Now it's good to go 👍. Let me know what else you need help with or what would make a difference, and I’ll be happy to assist. |
Add automatic hover effect detection and improve link handling in FwbListGroupItem
This commit enhances the FwbListGroupItem component with two key improvements:
Automatic hover styling when a click event handler is attached, without requiring an explicit prop. The component now checks for the presence of click event handlers in the component attributes.
Comprehensive link support with proper handling of:
hrefattributetoattributerouter-linkandnuxt-linktagsSummary by CodeRabbit
Documentation
href,to,target) instead of the removedhoverprop.New Features
href,to, andtargetprops, allowing for both external and router links.Refactor