-
Notifications
You must be signed in to change notification settings - Fork 138
feat: add Autocomplete component with examples and documentation #400
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: add Autocomplete component with examples and documentation #400
Conversation
Introduced the FwbAutocomplete component, enabling users to search and select options with real-time filtering. Added multiple examples demonstrating various functionalities, including remote data loading, custom option templates, and validation. Updated documentation to include the new component and its usage.
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FwbAutocomplete
participant ParentComponent
User->>FwbAutocomplete: Focuses input / types query
FwbAutocomplete->>FwbAutocomplete: Debounces input, filters options (local or remote)
alt Remote mode
FwbAutocomplete->>ParentComponent: Emits "search" event with query
ParentComponent-->>FwbAutocomplete: Updates options prop with results
end
FwbAutocomplete->>User: Shows filtered dropdown
User->>FwbAutocomplete: Selects option (click/keyboard)
FwbAutocomplete->>ParentComponent: Emits "update:modelValue" and "select" events
FwbAutocomplete->>User: Updates input and closes dropdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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 (
|
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: 9
🧹 Nitpick comments (6)
docs/components/autocomplete/examples/FwbAutocompleteExampleCustom.vue (2)
57-57
: Add proper TypeScript typing for consistency.The
selectedUser
ref should be typed consistently with other examples for better type safety.Apply this diff to improve type safety:
-const selectedUser = ref(null) +const selectedUser = ref<typeof users[0] | null>(null)
63-63
: Consider using local avatar images for documentation reliability.The external avatar URLs from pravatar.cc might not be reliable for documentation purposes and could fail in offline environments.
Consider using local placeholder images or generate simple avatar placeholders to ensure the documentation works consistently.
Also applies to: 69-69, 75-75, 81-81
docs/components/autocomplete/examples/FwbAutocompleteExampleRemote.vue (1)
75-111
: Consider more specific error messagesThe search function handles the API calls well with proper error handling and 404 status checks. However, the generic error message could be more informative.
Consider providing more specific error messages based on the error type:
} catch (err) { - error.value = 'Failed to search countries. Please try again.' + error.value = err instanceof Error ? err.message : 'Failed to search countries. Please try again.' countries.value = [] }docs/components/autocomplete.md (3)
24-49
: Fix markdown formatting: Add language specificationThe fenced code block should specify the language for proper syntax highlighting.
-``` +```vue
114-160
: Fix markdown formatting: Add language specificationThe fenced code block should specify the language for proper syntax highlighting.
-``` +```vue
205-254
: Fix markdown formatting: Add language specificationThe fenced code block should specify the language for proper syntax highlighting.
-``` +```vue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/.vitepress/config.ts
(1 hunks)docs/components/autocomplete.md
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExample.vue
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExampleCustom.vue
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExampleRemote.vue
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExampleSize.vue
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExampleValidation.vue
(1 hunks)src/components/FwbAutocomplete/FwbAutocomplete.vue
(1 hunks)src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts
(1 hunks)src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
(1 hunks)src/components/FwbAutocomplete/types.ts
(1 hunks)src/index.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/FwbAutocomplete/FwbAutocomplete.vue (1)
Learnt from: Sqrcz
PR: themesberg/flowbite-vue#338
File: docs/components/dropdown.md:243-261
Timestamp: 2025-01-11T13:26:22.855Z
Learning: The `disabled` prop in the FwbDropdown component is a boolean prop that defaults to `false` and controls whether the dropdown trigger button is disabled.
🧬 Code Graph Analysis (1)
src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts (1)
src/components/FwbAutocomplete/types.ts (2)
AutocompleteSize
(3-3)ValidationStatus
(4-4)
🪛 markdownlint-cli2 (0.17.2)
docs/components/autocomplete.md
49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
218-218: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
254-254: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (18)
docs/.vitepress/config.ts (1)
68-68
: LGTM! Navigation entry properly added.The Autocomplete component entry is correctly formatted and positioned alphabetically in the form components section.
src/index.ts (1)
6-6
: LGTM! Component export properly added.The FwbAutocomplete component export follows the established pattern and is correctly positioned alphabetically.
docs/components/autocomplete/examples/FwbAutocompleteExample.vue (1)
1-37
: LGTM! Well-structured basic example.The example effectively demonstrates the core functionality of the FwbAutocomplete component with proper TypeScript typing, v-model binding, and clear user feedback.
docs/components/autocomplete/examples/FwbAutocompleteExampleSize.vue (1)
1-53
: LGTM! Clear size variant demonstration.The example effectively shows all three size variants with proper spacing, labeling, and individual v-model bindings.
docs/components/autocomplete/examples/FwbAutocompleteExampleCustom.vue (1)
10-28
: Excellent custom option rendering example.The scoped slot implementation effectively demonstrates how to customize the dropdown options with rich content including avatars, names, and emails.
docs/components/autocomplete/examples/FwbAutocompleteExampleValidation.vue (2)
1-38
: LGTM: Well-structured validation exampleThe template demonstrates proper usage of validation slots and conditional rendering. The validation status binding and message templates are implemented correctly.
40-63
: LGTM: Clean composition API implementationThe script section properly uses Vue 3 Composition API with reactive references and computed properties. The validation logic is simple but effective for demonstration purposes.
docs/components/autocomplete/examples/FwbAutocompleteExampleRemote.vue (1)
1-54
: LGTM: Comprehensive remote search exampleThe template demonstrates proper usage of remote search features including loading states, custom option templates, and error handling. The slot customization with flag and region display is well-implemented.
docs/components/autocomplete.md (1)
256-293
: LGTM: Comprehensive API documentationThe API documentation provides clear information about props, events, and slots with proper type information and descriptions. This will be helpful for developers using the component.
src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts (1)
38-53
: LGTM: Clean utility functionsThe
getDropdownItemClasses
function andmessageClasses
computed property are well-implemented and provide the expected functionality for dynamic styling.src/components/FwbAutocomplete/types.ts (3)
1-5
: LGTM: Clean type imports and exportsThe imports from the input types module and re-exports are properly structured. The
AutocompleteSize
alias maintains consistency with the existing input component types.
6-27
: LGTM: Comprehensive props interfaceThe
AutocompleteProps
interface provides a complete set of configuration options with proper generic type support. The optional properties are well-defined and the types are consistent with the component's functionality.
29-33
: LGTM: Well-defined event emissionsThe
AutocompleteEmits
interface properly defines all component events with correct type signatures. The generic type parameter ensures type safety for the option data.src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (1)
1-23
: Test setup looks good!Proper use of fake timers for testing debounced functionality.
src/components/FwbAutocomplete/FwbAutocomplete.vue (4)
199-215
: Consider not clearing selection on every keystrokeThe
handleInput
method clears the modelValue immediately when typing begins. This might be unexpected behavior - users might want to refine their search while maintaining the current selection.Consider only clearing the selection when the user explicitly clears the input or selects a new option. This would provide a better user experience for incremental search refinement.
160-176
: Well-implemented filtering logic!The filtering implementation properly handles edge cases with optional chaining and nullish coalescing. Good defensive programming.
222-257
: Excellent keyboard navigation implementation!The keyboard handling is comprehensive with proper wrapping behavior and event prevention.
280-289
: Proper lifecycle management and cleanup!Good practice with removing event listeners and clearing timers on unmount.
…omplete component
…n with and without valueField
Hello guys! can you please review? Thanks! @wazeerc , @andreyjamer, @Sqrcz , @zoltanszogyenyi |
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.
Looking good, @Sqrcz will approve if aok
@moghaddam01 I looks nice, great job! But what I personally don't like is that the implementation is strictly tied to the input implementation. I mean internally it has the input that couldn't be customized. For example it is impossible to use prefix for icon instead of suffix. It is impossible to use a custom icon (from external icons set like fontawesome or something like this). Wouldn't it be better to pass the input to the slot of the component? Then developers will have a possibility to manage input appearance. And the only responsibility of the autocomplete will be to manage the list of options. Another thing that bothers me a little bit is the fact that the list is displayed using |
Thanks for your input @andreyjamer! |
Just pushed new commit , now the autocomplete component provides maximum flexibility by allowing you to use any Vue input component (default to FwbInput) through the |
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: 0
🔭 Outside diff range comments (1)
docs/components/autocomplete/examples/FwbAutocompleteExampleCustomInput.vue (1)
273-274
: Missing closing tag.The script tag appears to be missing its closing tag.
-</script> -274 +</script>
🧹 Nitpick comments (5)
docs/components/autocomplete.md (4)
25-50
: Address markdown formatting issue.The static analysis correctly identified a missing language specification for the code block.
-``` +```vue <template> <fwb-autocomplete v-model="selectedCountry"
158-204
: Fix markdown formatting issue.Missing language specification for the code block as flagged by static analysis.
-``` +```vue <template> <fwb-autocomplete v-model="selectedUser"
249-298
: Fix markdown formatting issue.Missing language specification for the code block.
-``` +```vue <template> <div class="space-y-4"> <fwb-autocomplete
362-401
: Fix heading level increment issue.The static analysis correctly identified that heading levels should only increment by one level at a time.
-#### Basic Usage +### Basic UsageAlso fix the missing language specification:
-``` +```vue <fwb-autocomplete v-model="selected"docs/components/autocomplete/examples/FwbAutocompleteExampleCustomInput.vue (1)
171-175
: Consider import path structure.The import from
../../../../src/index
suggests this example is tightly coupled to the source structure. Consider if this could be made more maintainable.-import { FwbAutocomplete } from '../../../../src/index' +import { FwbAutocomplete } from 'flowbite-vue'This would make the example more consistent with how users would actually import the component, though I understand this might be necessary for the documentation build process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/components/autocomplete.md
(1 hunks)docs/components/autocomplete/examples/FwbAutocompleteExampleCustomInput.vue
(1 hunks)src/components/FwbAutocomplete/FwbAutocomplete.vue
(1 hunks)src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
(1 hunks)src/components/FwbAutocomplete/types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/FwbAutocomplete/types.ts
- src/components/FwbAutocomplete/FwbAutocomplete.vue
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/autocomplete.md
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
262-262: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
298-298: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
375-375: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (8)
docs/components/autocomplete.md (3)
1-8
: Documentation setup looks good.The script setup properly imports all example components and follows Vue 3 composition API patterns.
112-154
: Excellent documentation of custom input flexibility.The custom input components section clearly demonstrates the component's flexibility and provides practical examples. The table format for props is well-organized and informative.
300-361
: Comprehensive API documentation.The API section provides excellent coverage of all props, events, and slots with clear descriptions and type information. This will be very helpful for developers using the component.
docs/components/autocomplete/examples/FwbAutocompleteExampleCustomInput.vue (5)
1-10
: Clean template structure.The template is well-organized with clear sections and proper semantic HTML structure.
12-77
: Excellent demonstration of custom input flexibility.This example effectively showcases the component's ability to accept custom input components, addressing one of the design concerns mentioned in the PR feedback. The custom suffix slot with loading states and clear functionality is well-implemented.
90-158
: Good example of input customization.The second example shows how to customize the default FwbInput with props, providing an alternative approach to the custom input component method.
177-241
: Well-designed custom input component.The custom input implementation is comprehensive and demonstrates advanced Vue 3 patterns:
- Proper prop definitions with TypeScript
- Correct event handling and emission
- Dynamic class composition
- Render function usage with
h()
- Accessible SVG icons with proper attributes
This addresses the PR feedback about input customization limitations by showing how developers can create fully custom input components.
258-272
: Clean TypeScript usage and state management.Good use of TypeScript for type safety and proper reactive state management with the composition API. The mock search function with timeout appropriately simulates real-world async behavior.
Thanks for your contribution! Awesome piece of work 🙌 I noticed that you're calling Why?
Example in FwbInput This approach also keeps class normalization logic centralized and consistent across the codebase. Thanks for updating this! Linter warns about few unused variables... but I wasn't able to check it properly yet... I'll play around a bit more with it on Monday and give you a proper update. Overall, this looks solid! 👌 |
Hi @Sqrcz, thanks for your comments! Here’s the checklist: Fix the Replace Let me know if I missed anything. |
Hej @moghaddam01, Thanks for your patience! I took some time to test the new component properly. Please update the PR according to your checklist. Additionally, I noticed a small issue in the documentation. In examples the Once these fixes are applied, I believe the PR will be ready for merging. For the next iteration we can think about making a multiselect out of it ;) Thanks for your work on this! |
please take a look at one more thing... when you tab in to this field... dropdown appears... when you tab out of it... dropdown stays... (it disappears when you click somewhere outside), but it should hide on blur. |
…ocomplete examples from a string to an array
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: 2
🧹 Nitpick comments (2)
docs/components/autocomplete.md (2)
124-131
: Keep prop casing consistent (inputComponent
vsinput-component
)Everywhere else, the docs adopt camelCase (
inputComponent
). Mixing kebab-case here may confuse users, especially TS users relying on autocomplete.- :input-component="CustomInput" + :inputComponent="CustomInput"Same for the second example below.
148-152
: Table header mismatch with actual prop nameThe table lists
inputComponent
/inputProps
correctly, but the first column is titled Prop whereas earlier tables use Name. Stick to one convention to avoid lint noise and keep auto-generated TOCs clean.-| Prop | Type | Default | Description | +| Name | Type | Default | Description |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/components/autocomplete.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/autocomplete.md
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
262-262: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
298-298: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
375-375: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (1)
docs/components/autocomplete.md (1)
24-26
: Add explicit language identifiers to all fenced code blocksSeveral blocks (detected by markdownlint MD040) open with ````` but omit the language tag, losing syntax highlighting.
-``` +```vueApplies to the blocks following the component demo lines in “Default”, “Remote Data with Debounce”, “Custom Option Template”, and “Validation” sections.
Also applies to: 52-56, 157-161, 206-210
const searchCountries = async (query) => { | ||
loading.value = true | ||
try { | ||
const response = await fetch(`https://restcountries.com/v3.1/name/${query}`) | ||
const data = await response.json() | ||
|
||
countries.value = data.slice(0, 10).map(country => ({ | ||
name: country.name.common, | ||
capital: country.capital?.[0] || 'N/A', |
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.
Encode the query to prevent malformed‐URL errors
searchCountries
interpolates the user’s raw query directly into the URL.
Spaces or non-ASCII chars will break the request (404 from RestCountries).
- const response = await fetch(`https://restcountries.com/v3.1/name/${query}`)
+ const response = await fetch(
+ `https://restcountries.com/v3.1/name/${encodeURIComponent(query)}`
+ )
Consider also checking response.ok
before response.json()
to surface HTTP errors clearly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const searchCountries = async (query) => { | |
loading.value = true | |
try { | |
const response = await fetch(`https://restcountries.com/v3.1/name/${query}`) | |
const data = await response.json() | |
countries.value = data.slice(0, 10).map(country => ({ | |
name: country.name.common, | |
capital: country.capital?.[0] || 'N/A', | |
const searchCountries = async (query) => { | |
loading.value = true | |
try { | |
const response = await fetch( | |
`https://restcountries.com/v3.1/name/${encodeURIComponent(query)}` | |
) | |
const data = await response.json() | |
countries.value = data.slice(0, 10).map(country => ({ | |
name: country.name.common, | |
capital: country.capital?.[0] || 'N/A', |
🤖 Prompt for AI Agents
In docs/components/autocomplete.md around lines 90 to 98, the searchCountries
function directly inserts the raw query into the URL, which can cause
malformed-URL errors with spaces or special characters. Fix this by encoding the
query parameter using encodeURIComponent before including it in the fetch URL.
Additionally, add a check for response.ok before calling response.json() to
handle HTTP errors explicitly and improve error reporting.
<fwb-autocomplete-example-size /> | ||
```vue | ||
<template> | ||
<div class="space-y-4"> | ||
<fwb-autocomplete | ||
v-model="selected1" | ||
:options="countries" | ||
:search-fields="['name']" | ||
display="name" | ||
label="Small" | ||
size="sm" | ||
placeholder="Small autocomplete..." | ||
/> | ||
|
||
<fwb-autocomplete | ||
v-model="selected2" | ||
:options="countries" | ||
:search-fields="['name']" | ||
display="name" | ||
label="Medium (default)" | ||
size="md" | ||
placeholder="Medium autocomplete..." | ||
/> | ||
|
||
<fwb-autocomplete | ||
v-model="selected3" | ||
:options="countries" | ||
:search-fields="['name']" | ||
display="name" | ||
label="Large" | ||
size="lg" | ||
placeholder="Large autocomplete..." | ||
/> | ||
</div> |
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.
Fix indented code block – markdownlint MD046
The “Sizes” example is rendered as an indented block instead of fenced, breaking syntax highlighting and copy-paste.
- <fwb-autocomplete
- v-model="selected1"
- :options="countries"
- …
- />
+```vue
+<fwb-autocomplete
+ v-model="selected1"
+ :options="countries"
+ …
+/>
+```
Repeat for the medium and large instances inside the same snippet.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
262-262: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🤖 Prompt for AI Agents
In docs/components/autocomplete.md between lines 248 and 281, the Vue code
snippet for the "Sizes" example is incorrectly formatted as an indented block
instead of a fenced code block, causing issues with syntax highlighting and
copy-paste. To fix this, replace the indented code block with a fenced code
block using triple backticks before and after the entire Vue template, ensuring
all three <fwb-autocomplete> instances are inside the fenced block with proper
indentation.
…onent during keyboard navigation
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
♻️ Duplicate comments (4)
docs/components/autocomplete.md (2)
90-98
: Encode the query to prevent malformed-URL errors
searchCountries
interpolates the user's raw query directly into the URL.
Spaces or non-ASCII chars will break the request (404 from RestCountries).- const response = await fetch(`https://restcountries.com/v3.1/name/${query}`) + const response = await fetch( + `https://restcountries.com/v3.1/name/${encodeURIComponent(query)}` + )Consider also checking
response.ok
beforeresponse.json()
to surface HTTP errors clearly.
248-281
: Fix indented code block – markdownlint MD046The "Sizes" example is rendered as an indented block instead of fenced, breaking syntax highlighting and copy-paste.
- <fwb-autocomplete - v-model="selected1" - :options="countries" - … - /> +```vue +<fwb-autocomplete + v-model="selected1" + :options="countries" + … +/> +```Repeat for the medium and large instances inside the same snippet.
src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts (1)
20-36
: Handle Record<string, boolean> class typesThe computed functions only handle string types from class props, but the interface allows
Record<string, boolean>
which is not being processed.const wrapperClasses = computed(() => { return useMergeClasses([ baseWrapperClasses, - typeof props.wrapperClass?.value === 'string' ? props.wrapperClass.value : '', + props.wrapperClass?.value || '', ]) })const dropdownClasses = computed(() => { return useMergeClasses([ baseDropdownClasses.value, - typeof props.dropdownClass?.value === 'string' ? props.dropdownClass.value : '', + props.dropdownClass?.value || '', ]) })The
useMergeClasses
function already handles normalization of different class input types including objects, so just pass the value directly.src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (1)
422-427
: Remove conditional to ensure test reliabilityconst firstOption = wrapper.find('[data-testid="fwb-autocomplete-option-0"]') -if (firstOption.exists()) { - await firstOption.trigger('click') - - // Input should show the selected option's display value - expect(input.element.value).toBe('United States') -} +expect(firstOption.exists()).toBe(true) +await firstOption.trigger('click') + +// Input should show the selected option's display value +expect(input.element.value).toBe('United States')
🧹 Nitpick comments (1)
docs/components/autocomplete.md (1)
363-376
: Fix heading level incrementThe heading jumps from h2 to h4, violating markdown best practices.
-#### Basic Usage +### Basic Usage-#### Remote Data with Custom Input Props +### Remote Data with Custom Input Props-#### Custom Input Component +### Custom Input Component
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/components/autocomplete.md
(1 hunks)src/components/FwbAutocomplete/FwbAutocomplete.vue
(1 hunks)src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts
(1 hunks)src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
(1 hunks)src/components/FwbAutocomplete/types.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/FwbAutocomplete/types.ts
🧰 Additional context used
🧠 Learnings (3)
src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts (1)
Learnt from: Sqrcz
PR: #389
File: src/components/FwbToggle/FwbToggle.vue:0-0
Timestamp: 2025-07-18T12:11:01.047Z
Learning: In the FwbToggle component, the toggleBallClasses
variable name is misleading - it doesn't style the actual toggle ball/thumb, but rather the label text container. The visual toggle ball is rendered through other means (likely CSS on the toggle track or pseudo-elements).
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (2)
Learnt from: Sqrcz
PR: #338
File: docs/components/dropdown.md:243-261
Timestamp: 2025-01-11T13:26:22.855Z
Learning: The disabled
prop in the FwbDropdown component is a boolean prop that defaults to false
and controls whether the dropdown trigger button is disabled.
Learnt from: Sqrcz
PR: #389
File: src/components/FwbToggle/FwbToggle.vue:0-0
Timestamp: 2025-07-18T12:11:01.047Z
Learning: In the FwbToggle component, the toggleBallClasses
variable name is misleading - it doesn't style the actual toggle ball/thumb, but rather the label text container. The visual toggle ball is rendered through other means (likely CSS on the toggle track or pseudo-elements).
src/components/FwbAutocomplete/FwbAutocomplete.vue (2)
Learnt from: Sqrcz
PR: #338
File: src/components/FwbDropdown/FwbDropdown.vue:9-9
Timestamp: 2025-01-11T13:26:22.708Z
Learning: In Vue components with disabled states, ensure click handlers respect the disabled prop by adding early returns (e.g., if (props.disabled) return
). The disabled attribute alone on elements isn't sufficient as the click handler can still execute.
Learnt from: Sqrcz
PR: #389
File: src/components/FwbToggle/FwbToggle.vue:0-0
Timestamp: 2025-07-18T12:11:01.047Z
Learning: In the FwbToggle component, the toggleBallClasses
variable name is misleading - it doesn't style the actual toggle ball/thumb, but rather the label text container. The visual toggle ball is rendered through other means (likely CSS on the toggle track or pseudo-elements).
🧬 Code Graph Analysis (1)
src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts (1)
src/composables/useMergeClasses.ts (1)
useMergeClasses
(27-29)
🪛 markdownlint-cli2 (0.17.2)
docs/components/autocomplete.md
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
204-204: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
262-262: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
298-298: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
376-376: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (2)
src/components/FwbAutocomplete/FwbAutocomplete.vue (2)
111-114
: Well-implemented dropdown state managementThe use of
isSelectingOption
flag with mousedown/mouseup/mouseleave events effectively prevents the dropdown from closing during option selection, solving the common blur/click race condition.Also applies to: 271-274
330-344
: Robust scroll handling implementationGood defensive programming by checking if
scrollIntoView
is a function before calling it, ensuring compatibility across different environments.
const baseDropdownClasses = computed(() => | ||
`absolute z-[${props.zIndex?.value}] w-full mt-1 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 rounded-lg shadow-lg max-h-60 overflow-y-auto`, | ||
) |
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.
Avoid dynamic Tailwind class generation
Using template literals to generate Tailwind classes like z-[${props.zIndex?.value}]
is problematic because Tailwind's purge mechanism won't detect these dynamically generated classes.
Use inline styles for dynamic values instead:
-const baseDropdownClasses = computed(() =>
- `absolute z-[${props.zIndex?.value}] w-full mt-1 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 rounded-lg shadow-lg max-h-60 overflow-y-auto`,
-)
+const baseDropdownClasses = 'absolute w-full mt-1 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 rounded-lg shadow-lg max-h-60 overflow-y-auto'
Then apply z-index as an inline style in the component template:
<div :class="dropdownClasses" :style="{ zIndex: zIndex }">
🤖 Prompt for AI Agents
In src/components/FwbAutocomplete/composables/useAutocompleteClasses.ts around
lines 27 to 29, avoid using template literals to dynamically generate Tailwind
classes like z-[${props.zIndex?.value}] because Tailwind's purge mechanism
cannot detect them. Instead, remove the dynamic z-index class from the computed
class string and apply the z-index value as an inline style in the component
template by binding :style="{ zIndex: props.zIndex?.value }" alongside the
static classes.
if (firstOption.exists()) { | ||
await firstOption.trigger('click') | ||
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | ||
expect(wrapper.emitted('select')).toBeTruthy() | ||
} | ||
}) |
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.
Remove conditional to ensure test reliability
The conditional could cause the test to pass even when no dropdown items are found.
const firstOption = wrapper.find('[data-testid="fwb-autocomplete-option-0"]')
-if (firstOption.exists()) {
- await firstOption.trigger('click')
- expect(wrapper.emitted('update:modelValue')).toBeTruthy()
- expect(wrapper.emitted('select')).toBeTruthy()
-}
+expect(firstOption.exists()).toBe(true)
+await firstOption.trigger('click')
+expect(wrapper.emitted('update:modelValue')).toBeTruthy()
+expect(wrapper.emitted('select')).toBeTruthy()
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (firstOption.exists()) { | |
await firstOption.trigger('click') | |
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | |
expect(wrapper.emitted('select')).toBeTruthy() | |
} | |
}) | |
const firstOption = wrapper.find('[data-testid="fwb-autocomplete-option-0"]') | |
expect(firstOption.exists()).toBe(true) | |
await firstOption.trigger('click') | |
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | |
expect(wrapper.emitted('select')).toBeTruthy() | |
}) |
🤖 Prompt for AI Agents
In src/components/FwbAutocomplete/tests/Autocomplete.spec.ts around lines 89 to
94, remove the conditional check that verifies if firstOption.exists() before
triggering the click and asserting emitted events. This ensures the test always
attempts to click the first option and properly fails if no dropdown items are
present, improving test reliability.
if (secondOption.exists()) { | ||
await secondOption.trigger('mouseenter') | ||
|
||
// Check that the option is highlighted (this would be internal state) | ||
// We can verify by checking classes or other visual indicators | ||
expect(secondOption.classes()).toContain('fwb-autocomplete-option') | ||
} |
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.
Remove conditional and improve assertion
The test only verifies that a CSS class exists but doesn't actually verify highlighting behavior.
const secondOption = wrapper.find('[data-testid="fwb-autocomplete-option-1"]')
-if (secondOption.exists()) {
- await secondOption.trigger('mouseenter')
-
- // Check that the option is highlighted (this would be internal state)
- // We can verify by checking classes or other visual indicators
- expect(secondOption.classes()).toContain('fwb-autocomplete-option')
-}
+expect(secondOption.exists()).toBe(true)
+await secondOption.trigger('mouseenter')
+
+// Verify the option has highlight styling
+expect(secondOption.classes()).toContain('bg-blue-50')
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (secondOption.exists()) { | |
await secondOption.trigger('mouseenter') | |
// Check that the option is highlighted (this would be internal state) | |
// We can verify by checking classes or other visual indicators | |
expect(secondOption.classes()).toContain('fwb-autocomplete-option') | |
} | |
const secondOption = wrapper.find('[data-testid="fwb-autocomplete-option-1"]') | |
expect(secondOption.exists()).toBe(true) | |
await secondOption.trigger('mouseenter') | |
// Verify the option has highlight styling | |
expect(secondOption.classes()).toContain('bg-blue-50') |
🤖 Prompt for AI Agents
In src/components/FwbAutocomplete/tests/Autocomplete.spec.ts between lines 399
and 405, remove the conditional check for secondOption.exists() to ensure the
test always runs. Replace the current assertion that only checks for the
presence of a CSS class with a more specific assertion that verifies the option
is actually highlighted, such as checking for a distinct highlight class or
style that indicates the option is selected or focused.
Hi guys – pushed some new commits covering all your inputs:
Thanks for all your valuable inputs and comments! |
Any update? @Sqrcz |
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: 1
♻️ Duplicate comments (3)
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (3)
89-94
: Remove conditional to ensure test reliability.The conditional could cause the test to pass even when no dropdown items are found.
const firstOption = wrapper.find('[data-testid="fwb-autocomplete-option-0"]') -if (firstOption.exists()) { - await firstOption.trigger('click') - expect(wrapper.emitted('update:modelValue')).toBeTruthy() - expect(wrapper.emitted('select')).toBeTruthy() -} +expect(firstOption.exists()).toBe(true) +await firstOption.trigger('click') +expect(wrapper.emitted('update:modelValue')).toBeTruthy() +expect(wrapper.emitted('select')).toBeTruthy()
399-405
: Remove conditional and improve assertion.The test only verifies that a CSS class exists but doesn't actually verify highlighting behavior.
const secondOption = wrapper.find('[data-testid="fwb-autocomplete-option-1"]') -if (secondOption.exists()) { - await secondOption.trigger('mouseenter') - - // Check that the option is highlighted (this would be internal state) - // We can verify by checking classes or other visual indicators - expect(secondOption.classes()).toContain('fwb-autocomplete-option') -} +expect(secondOption.exists()).toBe(true) +await secondOption.trigger('mouseenter') + +// Verify the option has highlight styling +expect(secondOption.classes()).toContain('bg-blue-50')
422-427
: Remove conditional to ensure test reliability.The conditional could cause the test to pass even when no dropdown items are found.
const firstOption = wrapper.find('[data-testid="fwb-autocomplete-option-0"]') -if (firstOption.exists()) { - await firstOption.trigger('click') - - // Input should show the selected option's display value - expect(input.element.value).toBe('United States') -} +expect(firstOption.exists()).toBe(true) +await firstOption.trigger('click') + +// Input should show the selected option's display value +expect(input.element.value).toBe('United States')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: the `disabled` prop in the fwbdropdown component is a boolean prop that defaults to `false` and cont...
Learnt from: Sqrcz
PR: themesberg/flowbite-vue#338
File: docs/components/dropdown.md:243-261
Timestamp: 2025-01-11T13:26:22.855Z
Learning: The `disabled` prop in the FwbDropdown component is a boolean prop that defaults to `false` and controls whether the dropdown trigger button is disabled.
Applied to files:
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
📚 Learning: in the fwbtoggle component, the `toggleballclasses` variable name is misleading - it doesn't style t...
Learnt from: Sqrcz
PR: themesberg/flowbite-vue#389
File: src/components/FwbToggle/FwbToggle.vue:0-0
Timestamp: 2025-07-18T12:11:01.047Z
Learning: In the FwbToggle component, the `toggleBallClasses` variable name is misleading - it doesn't style the actual toggle ball/thumb, but rather the label text container. The visual toggle ball is rendered through other means (likely CSS on the toggle track or pseudo-elements).
Applied to files:
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build-pkg (18.x)
- GitHub Check: build-types (20.x)
- GitHub Check: build-pkg (20.x)
- GitHub Check: lint (18.x)
- GitHub Check: docs-build (22.x)
- GitHub Check: docs-build (20.x)
- GitHub Check: docs-build (18.x)
🔇 Additional comments (23)
src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (23)
1-23
: LGTM! Well-structured test setup.The imports, mock data, and fake timer setup/teardown are properly configured for comprehensive component testing.
24-37
: LGTM! Clean basic rendering test.The test properly verifies that basic props are rendered correctly in the component.
39-56
: LGTM! Comprehensive filtering test.The test effectively verifies both positive and negative filtering results with proper async handling.
58-72
: LGTM! Proper dropdown visibility test.The test correctly verifies that the dropdown appears on input focus using reliable data-testid selectors.
96-113
: LGTM! Solid keyboard navigation test.The test properly verifies keyboard navigation functionality without conditional checks that could hide failures.
134-151
: LGTM! Comprehensive loading state test.The test thoroughly verifies loading state rendering including text, spinner, and message elements using reliable selectors.
153-169
: LGTM! Effective no results test.The test properly verifies the no results state by simulating a search with no matches and checking for the appropriate message and element.
171-194
: LGTM! Excellent debounce testing.The test effectively uses fake timers to verify debounce behavior, ensuring search events are properly delayed and then emitted with correct parameters.
196-210
: LGTM! Proper slot testing.The test effectively verifies that validation message slots are rendered correctly with appropriate content and elements.
212-225
: LGTM! Consistent slot testing.The test properly verifies helper text slot rendering following the same reliable pattern as other slot tests.
227-239
: LGTM! Proper prop passing test.The test effectively verifies that size props are correctly passed to the child FwbInput component.
241-251
: LGTM! Clear disabled state test.The test directly verifies that the disabled prop is properly applied to the input element.
253-271
: LGTM! Effective keyboard interaction test.The test properly verifies that the Escape key closes the dropdown, checking both the open and closed states.
273-288
: LGTM! Comprehensive multi-field search test.The test effectively verifies that searching works across multiple fields by searching for a code that matches a country.
290-304
: LGTM! Proper function mocking test.The test effectively uses vi.fn() to verify that custom display functions are called with the correct parameters.
306-337
: LGTM! Comprehensive valueField test.The test thoroughly verifies that options render and select correctly when valueField is specified, with proper event emission verification.
339-369
: LGTM! Complete default behavior test.The test effectively verifies that the component works correctly without valueField, maintaining proper selection and emission behavior.
371-383
: LGTM! Clear focus event test.The test properly verifies that search events are emitted when the input receives focus.
430-450
: LGTM! Proper blur behavior test.The test effectively verifies that the dropdown closes on blur, checking both open and closed states with proper async handling.
452-472
: LGTM! Thorough props passing test.The test effectively verifies that inputProps are correctly passed to the child FwbInput component with proper object matching.
474-485
: LGTM! Clear default component test.The test properly verifies that FwbInput is used as the default input component.
487-504
: LGTM! Effective z-index customization test.The test properly verifies that custom z-index values are applied to the dropdown with the correct CSS class.
506-522
: LGTM! Complete z-index testing.The test properly verifies that the default z-index is applied when no custom value is specified, providing comprehensive z-index coverage.
if (clearButton.exists()) { | ||
await clearButton.trigger('click') | ||
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | ||
const emittedEvents = wrapper.emitted('update:modelValue') as Array<any> | ||
expect(emittedEvents[emittedEvents.length - 1][0]).toBe(null) | ||
} |
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.
Remove conditional to ensure test reliability.
The conditional could cause the test to pass even when the clear button is not found.
const clearButton = wrapper.find('[data-testid="fwb-autocomplete-clear-button"]')
-if (clearButton.exists()) {
- await clearButton.trigger('click')
- expect(wrapper.emitted('update:modelValue')).toBeTruthy()
- const emittedEvents = wrapper.emitted('update:modelValue') as Array<any>
- expect(emittedEvents[emittedEvents.length - 1][0]).toBe(null)
-}
+expect(clearButton.exists()).toBe(true)
+await clearButton.trigger('click')
+expect(wrapper.emitted('update:modelValue')).toBeTruthy()
+const emittedEvents = wrapper.emitted('update:modelValue') as Array<any>
+expect(emittedEvents[emittedEvents.length - 1][0]).toBe(null)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (clearButton.exists()) { | |
await clearButton.trigger('click') | |
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | |
const emittedEvents = wrapper.emitted('update:modelValue') as Array<any> | |
expect(emittedEvents[emittedEvents.length - 1][0]).toBe(null) | |
} | |
const clearButton = wrapper.find('[data-testid="fwb-autocomplete-clear-button"]') | |
expect(clearButton.exists()).toBe(true) | |
await clearButton.trigger('click') | |
expect(wrapper.emitted('update:modelValue')).toBeTruthy() | |
const emittedEvents = wrapper.emitted('update:modelValue') as Array<any> | |
expect(emittedEvents[emittedEvents.length - 1][0]).toBe(null) |
🤖 Prompt for AI Agents
In src/components/FwbAutocomplete/tests/Autocomplete.spec.ts around lines 126 to
131, remove the if conditional that checks if clearButton.exists() so the test
always attempts to trigger the click on the clear button. This ensures the test
fails if the clear button is not found, improving test reliability by not
silently passing when the button is missing.
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.
Awesome, thank you for all the updates.
Great work. 👍
Introduced the FwbAutocomplete component, enabling users to search and select options with real-time filtering. Added multiple examples demonstrating various functionalities, including remote data loading, custom option templates, and validation. Updated documentation to include the new component and its usage.
Summary by CodeRabbit
New Features
Tests