-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(autocomplete): add autoHighlight prop #3829
base: canary
Are you sure you want to change the base?
Conversation
…ht the first item in the list
🦋 Changeset detectedLatest commit: 2a313e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@dgz9 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces an autocomplete component that allows users to select animals from a predefined dataset. Key features include the addition of an Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 2
🧹 Outside diff range and nitpick comments (10)
apps/docs/content/components/autocomplete/auto-highlight.ts (1)
29-44
: RefactorApp
component and approveisAutoHighlight
usageThe
App
component is currently defined as a string, which is not a standard practice and could lead to issues:
- It prevents proper syntax highlighting and code analysis tools from working correctly.
- It makes the code harder to maintain and update.
However, the usage of the new
isAutoHighlight
prop is correct and aligns with the PR objectives.Consider refactoring the
App
component as follows:import React from 'react'; import { Autocomplete, AutocompleteItem } from "@nextui-org/react"; import { animals } from "./data"; export default function App() { return ( <Autocomplete isAutoHighlight label="Favorite Animal" placeholder="Search an animal" className="max-w-xs" defaultItems={animals} > {(item) => <AutocompleteItem key={item.value}>{item.label}</AutocompleteItem>} </Autocomplete> ); }The usage of
isAutoHighlight
prop is correct and implements the new feature as intended.packages/components/listbox/src/listbox.tsx (2)
56-56
: LGTM: Implementation ofisAutoHighlighted
in ListboxItemThe
isAutoHighlighted
prop is correctly implemented in the ListboxItem component. The conditionisAutoHighlight && state.selectionManager.focusedKey === item.key
ensures that only the first item is highlighted when the feature is enabled, which aligns with the PR objectives.For improved clarity, consider renaming the prop to
isAutoHighlighted
(past tense) in the ListboxItem component to better reflect its state:- isAutoHighlighted={isAutoHighlight && state.selectionManager.focusedKey === item.key} + isAutoHighlighted={isAutoHighlight && state.selectionManager.focusedKey === item.key}This change would make it clearer that this prop represents the computed state rather than the feature flag.
27-27
: Overall impact: Well-implemented feature with minimal changesThe implementation of the
isAutoHighlight
feature is well-done, with minimal and focused changes. It enhances the Listbox component's usability without breaking existing functionality. The feature is opt-in, ensuring backward compatibility.To complete this feature implementation:
- Ensure that the
UseListboxProps
interface (likely in a separate file) includes theisAutoHighlight
prop with appropriate JSDoc comments.- Update the component's documentation to explain the new
isAutoHighlight
prop, its default value, and its effect on the Listbox behavior.- Consider adding a unit test to verify the auto-highlight behavior when the prop is enabled.
Also applies to: 56-56
packages/components/listbox/src/use-listbox-item.ts (2)
116-117
: LGTM with a suggestion: Consider refactoring for readabilityThe
isHighlighted
logic has been correctly updated to include the newisAutoHighlighted
prop. The implementation is correct and maintains the existing functionality.To improve readability, consider refactoring the condition into separate variables:
const isHighlightedOnFocus = shouldHighlightOnFocus && isFocused; const isHighlightedOnInteraction = isMobile ? isHovered || isPressed : isHovered || (isFocused && !isFocusVisible); const isHighlighted = isHighlightedOnFocus || isHighlightedOnInteraction || isAutoHighlighted;This refactoring would make the logic easier to understand and maintain.
Action Required: Add Tests, Stories, and Documentation for
isAutoHighlighted
The
isAutoHighlighted
prop has been successfully implemented in theuse-listbox-item.ts
file. However, it is not currently utilized in any test files or story examples, and there is no corresponding documentation.To ensure the new prop functions as intended and is properly communicated to users, please:
Tests:
- Add unit tests that cover various scenarios involving the
isAutoHighlighted
prop.Stories:
- Create or update storybook stories to demonstrate the usage and behavior of the
isAutoHighlighted
prop.Documentation:
- Document the
isAutoHighlighted
prop in the relevant component guides or API documentation.Addressing these areas will enhance the reliability and usability of the new prop.
🔗 Analysis chain
Line range hint
1-190
: Summary: Successfully implementedisAutoHighlighted
functionalityThe changes in this file successfully implement the new
isAutoHighlighted
prop for the Autocomplete component. The implementation:
- Adds the new prop to the
Props
interface.- Sets a default value of
false
when destructuring props.- Incorporates the new prop into the
isHighlighted
logic.These changes align with the PR objectives and maintain backwards compatibility. The code style is consistent with the existing codebase.
To ensure the changes don't introduce any unintended side effects, please run the following verification script:
This script will help verify that the new prop is being used correctly in tests, stories, and documentation.
To accurately verify the usage of the
isAutoHighlighted
prop, please run the updated verification scripts below:This revised script uses
-g
to specify file patterns, ensuring thatrg
correctly identifies and searches through.test.ts
,.test.tsx
,.stories.ts
,.stories.tsx
,.md
, and.mdx
files. It also gracefully handles cases where documentation files might not exist by providing a fallback message.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of isAutoHighlighted prop in the codebase # Test 1: Check for any usages of isAutoHighlighted in test files echo "Checking for isAutoHighlighted usage in test files:" rg --type ts --type tsx 'isAutoHighlighted' packages/**/*.test.{ts,tsx} # Test 2: Check for any usages of isAutoHighlighted in story files echo "Checking for isAutoHighlighted usage in story files:" rg --type ts --type tsx 'isAutoHighlighted' packages/**/*.stories.{ts,tsx} # Test 3: Check for any usages of isAutoHighlighted in documentation files echo "Checking for isAutoHighlighted in documentation:" rg 'isAutoHighlighted' docs/**/*.{md,mdx}Length of output: 743
Script:
# #!/bin/bash # Description: Verify the usage of isAutoHighlighted prop in the codebase # Test 1: Check for any usages of isAutoHighlighted in test files echo "Checking for isAutoHighlighted usage in test files:" rg -g "*.test.ts" -g "*.test.tsx" 'isAutoHighlighted' packages/ # Test 2: Check for any usages of isAutoHighlighted in story files echo "Checking for isAutoHighlighted usage in story files:" rg -g "*.stories.ts" -g "*.stories.tsx" 'isAutoHighlighted' packages/ # Test 3: Check for any usages of isAutoHighlighted in documentation files echo "Checking for isAutoHighlighted in documentation:" rg -g "*.md" -g "*.mdx" 'isAutoHighlighted' docs/ || echo "No documentation files found."Length of output: 633
packages/components/listbox/src/use-listbox.ts (1)
95-99
: LGTM! Consider enhancing the documentation.The addition of the
isAutoHighlight
prop is well-implemented and aligns with the PR objectives. The prop is correctly typed as an optional boolean with a clear and concise documentation.Consider enhancing the documentation to provide more context about the prop's behavior:
/** * Whether to automatically highlight the first item in the list. + * When true, the first item in the dropdown will be highlighted as the user types. * @default false */ isAutoHighlight?: boolean;
packages/components/autocomplete/src/use-autocomplete.ts (2)
113-117
: LGTM! Consider enhancing the JSDoc comment.The addition of the
isAutoHighlight
property is well-implemented. The type is correct, and the default value is clearly stated.Consider adding a brief explanation of the potential impact on user experience in the JSDoc comment. For example:
/** * Whether to automatically highlight the first item in the list as the user types. * This can improve keyboard navigation efficiency for users. * @default false */ isAutoHighlight?: boolean;
339-345
: LGTM! Consider a minor optimization.The implementation of the
useEffect
hook for theisAutoHighlight
feature is correct and well-structured. It properly handles the auto-highlighting of the first item based on the specified conditions.Consider memoizing the
state.collection.size
to potentially optimize performance:const collectionSize = useMemo(() => state.collection.size, [state.collection]); useEffect(() => { if (isAutoHighlight && isOpen && collectionSize > 0) { const firstKey = state.collection.getFirstKey(); state.selectionManager.setFocusedKey(firstKey); } }, [isAutoHighlight, isOpen, state.inputValue, collectionSize]);This change could help avoid unnecessary recalculations of the collection size.
packages/components/autocomplete/stories/autocomplete.stories.tsx (1)
811-824
: LGTM: NewAutoHighlightTemplate
added.The
AutoHighlightTemplate
has been implemented correctly to demonstrate theisAutoHighlight
feature. It uses theisAutoHighlight
prop and sets it totrue
, showcasing the new functionality.A minor suggestion for improvement:
Consider adding a comment explaining the purpose of the
isAutoHighlight
prop for better documentation. For example:const AutoHighlightTemplate = ({color, variant, ...args}: AutocompleteProps<Animal>) => ( <Autocomplete - isAutoHighlight + isAutoHighlight // Automatically highlight the first item in the dropdown className="max-w-xs" color={color} defaultItems={animalsData} label="Favorite Animal" placeholder="Select an animal" variant={variant} {...args} > {(item) => <AutocompleteItem key={item.value}>{item.label}</AutocompleteItem>} </Autocomplete> );apps/docs/content/docs/components/autocomplete.mdx (1)
325-325
: Minor formatting suggestion for consistencyThe header for the new "Auto Highlight" section should be on a separate line for consistency with other sections in the document.
Consider applying this change:
-### Auto Highlight + +### Auto Highlight🧰 Tools
🪛 LanguageTool
[uncategorized] ~325-~325: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...={autocompleteContent.sections} /> ### Auto Highlight If you pass theisAutoHighlight
prop...(AUTO_HYPHEN)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- apps/docs/content/components/autocomplete/auto-highlight.ts (1 hunks)
- apps/docs/content/components/autocomplete/index.ts (2 hunks)
- apps/docs/content/docs/components/autocomplete.mdx (2 hunks)
- packages/components/autocomplete/src/use-autocomplete.ts (5 hunks)
- packages/components/autocomplete/stories/autocomplete.stories.tsx (3 hunks)
- packages/components/listbox/src/listbox.tsx (2 hunks)
- packages/components/listbox/src/use-listbox-item.ts (4 hunks)
- packages/components/listbox/src/use-listbox.ts (3 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/autocomplete.mdx
[uncategorized] ~325-~325: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...={autocompleteContent.sections} /> ### Auto Highlight If you pass theisAutoHighlight
prop...(AUTO_HYPHEN)
🔇 Additional comments (17)
apps/docs/content/components/autocomplete/auto-highlight.ts (1)
46-53
: Clarify the purpose of the export structureThe current export structure is unusual:
- It uses file paths as keys in the
react
object.- The entire module is exported as an object spread of
react
.This structure might be intentional for a specific use case (e.g., documentation or example system), but it's not a typical pattern for React components or data modules.
Could you please clarify the intended use of this export structure? Is it specifically designed for a documentation system or another particular use case?
If this is not intentional or required by the project structure, consider simplifying the exports:
export { animals }; export { default as App } from './App';This would make the module more straightforward and easier to import in other parts of the application.
packages/components/listbox/src/listbox.tsx (1)
27-27
: LGTM: Addition ofisAutoHighlight
propThe
isAutoHighlight
prop has been correctly added to the destructured props from theuseListbox
hook. This addition aligns with the PR objectives and follows the existing naming conventions for boolean props in the component.packages/components/listbox/src/use-listbox-item.ts (2)
24-24
: LGTM: New prop added correctlyThe
isAutoHighlighted
prop has been added to theProps
interface as an optional boolean. This addition aligns with the PR objectives and follows the existing naming conventions.
50-50
: LGTM: Default value set appropriatelyThe
isAutoHighlighted
prop is correctly destructured with a default value offalse
. This ensures backwards compatibility and follows the existing code structure.packages/components/listbox/src/use-listbox.ts (3)
126-126
: LGTM! Correct implementation of the new prop.The
isAutoHighlight
prop is correctly added to theuseListbox
function parameters with the default value matching the documentation. This implementation aligns well with the PR objectives.
Line range hint
1-195
: Overall, excellent implementation of the newisAutoHighlight
prop!The changes in this file successfully implement the
isAutoHighlight
prop as described in the PR objectives. The prop is well-documented, correctly typed, and properly integrated into theuseListbox
hook. These changes provide a solid foundation for the new auto-highlight feature in the Autocomplete component.A few suggestions to consider:
- Enhance the prop documentation for better clarity.
- Verify the usage of this new prop in dependent components, especially the Autocomplete component.
Great job on maintaining code quality and ensuring backwards compatibility!
190-190
: LGTM! Verify usage in dependent components.The
isAutoHighlight
prop is correctly added to the return object of theuseListbox
function, making it available to components using this hook. This change is necessary and aligns with the PR objectives.To ensure the new prop is properly utilized, please verify its usage in dependent components, particularly in the Autocomplete component mentioned in the PR objectives. Run the following script to check for potential usage:
packages/components/autocomplete/src/use-autocomplete.ts (4)
173-173
: LGTM! Correct implementation of the new prop.The
isAutoHighlight
prop is correctly added to the destructured props with the appropriate default value offalse
, matching the interface definition.
440-440
: LGTM! Correct propagation of theisAutoHighlight
prop.The
isAutoHighlight
prop is correctly added to thegetListBoxProps
function's return object. This ensures that the ListBox component receives this information, allowing it to adjust its behavior based on the auto-highlight feature.
524-524
: LGTM! Proper exposure of theisAutoHighlight
prop.The
isAutoHighlight
prop is correctly added to the return object of theuseAutocomplete
hook. This allows components consuming this hook to access theisAutoHighlight
value, enabling them to implement additional logic or rendering based on this feature if needed.
Line range hint
1-536
: Verify the impact on component behavior and performance.The implementation of the
isAutoHighlight
feature looks correct and complete. However, it's important to ensure that this addition doesn't negatively impact the overall behavior or performance of the Autocomplete component.Please run the following script to check for any potential performance regressions:
Additionally, please ensure that comprehensive tests have been added to cover various scenarios with the
isAutoHighlight
prop enabled and disabled.✅ Verification successful
Verified the impact on component behavior and performance.
No performance issues related to the
isAutoHighlight
feature were found. Additionally, the TODO regardingdisableClearable
is unrelated to this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential performance issues related to the new isAutoHighlight feature # Test 1: Check if isAutoHighlight is used in any tight loops or frequently called functions echo "Checking for potential performance issues:" rg -n 'isAutoHighlight' packages/components/autocomplete/src --type ts # Test 2: Verify that isAutoHighlight is properly memoized if used in dependency arrays echo "Checking if isAutoHighlight is properly memoized:" rg -n 'isAutoHighlight.*\[' packages/components/autocomplete/src --type ts # Test 3: Look for any TODOs or FIXMEs related to the new feature echo "Checking for any remaining TODOs or FIXMEs:" rg -n 'TODO|FIXME' packages/components/autocomplete/src --type tsLength of output: 1247
packages/components/autocomplete/stories/autocomplete.stories.tsx (2)
68-72
: LGTM: NewisAutoHighlight
property added toargTypes
.The
isAutoHighlight
property has been correctly added to theargTypes
object with a boolean control type. This allows users to toggle the auto-highlight feature in the Storybook interface.
1085-1090
: LGTM: NewWithAutoHighlight
export added.The
WithAutoHighlight
export has been correctly added to showcase the newAutoHighlightTemplate
in the storybook. This will allow users to interact with and test the newisAutoHighlight
feature.apps/docs/content/docs/components/autocomplete.mdx (4)
326-329
: New feature documentation looks goodThe new "Auto Highlight" section effectively introduces the
isAutoHighlight
property and provides a code demo for users to reference. This addition enhances the documentation by clearly explaining a new feature of the Autocomplete component.
87-88
: Improved clarity forisReadOnly
propertyThe description for the
isReadOnly
property has been updated to provide more clarity on its behavior. This change helps users better understand how the Autocomplete component functions when in a read-only state.
327-328
: API documentation updated with new featureThe
isAutoHighlight
property has been correctly added to the API documentation, maintaining consistency with the newly introduced feature. This ensures that users have a complete reference for all available properties of the Autocomplete component.
Line range hint
1-1
: Summary of documentation updatesThe Autocomplete component documentation has been successfully updated with the following key changes:
- Introduction of the new "Auto Highlight" feature, complete with an explanation and code demo.
- Addition of the
isAutoHighlight
property to the API documentation.- Improved clarity for the
isReadOnly
property description.These updates enhance the overall quality and completeness of the documentation, providing users with accurate and up-to-date information about the Autocomplete component's features and usage.
@@ -26,6 +26,7 @@ import customSectionsStyle from "./custom-sections-style"; | |||
import customStyles from "./custom-styles"; | |||
import customEmptyContentMessage from "./custom-empty-content-message"; | |||
import readOnly from "./read-only"; | |||
import isAutoHighlight from "./auto-highlight"; |
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.
💡 Codebase verification
Issue Found: auto-highlight.ts
Does Not Contain isAutoHighlight
Implementation
The auto-highlight.ts
file exports unrelated data and components instead of the expected isAutoHighlight
functionality. Please review and ensure that isAutoHighlight
is properly defined and exported within this file.
🔗 Analysis chain
LGTM! The changes align with the PR objectives.
The addition of isAutoHighlight
to the imports and autocompleteContent
object is consistent with the existing code structure and implements the new feature as described in the PR objectives.
Let's verify the existence and content of the auto-highlight.ts
file:
Also applies to: 60-60
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and content of the auto-highlight.ts file
# Test 1: Check if the file exists
if fd -t f "auto-highlight.ts" apps/docs/content/components/autocomplete; then
echo "auto-highlight.ts file exists."
else
echo "Error: auto-highlight.ts file not found."
exit 1
fi
# Test 2: Check the content of the file
cat apps/docs/content/components/autocomplete/auto-highlight.ts
Length of output: 2104
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.
- the main logic would be taken in feat(autocomplete): automatically focus first non-disabled item #2186. Please sync the latest canary.
- please add tests for that
- please add changeset
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 and nitpick comments (2)
.changeset/angry-pillows-accept.md (1)
1-6
: LGTM! Minor suggestion for improved clarity.The changeset file correctly identifies this as a minor version change for both "@nextui-org/autocomplete" and "@nextui-org/listbox" packages, which is appropriate for adding a new feature in a backwards-compatible manner.
Consider slightly expanding the change description for improved clarity:
- add prop isAutoHighlight to enable/disable the automatic focus on autocomplete items (#2186) + add prop isAutoHighlight to enable/disable the automatic focus on the first item in autocomplete and listbox components (#2186)This modification clarifies that the prop affects the first item and explicitly mentions both affected components.
packages/components/autocomplete/__tests__/autocomplete.test.tsx (1)
912-935
: LGTM with a minor suggestion: Robust test for disabled auto-highlight.This test case effectively verifies that no items are auto-highlighted when the
isAutoHighlight
prop is set to false. The test structure is clear and follows best practices.Consider adding an assertion to check that the listbox is actually open before verifying that no options are highlighted. This would make the test more robust by ensuring the expected state is reached before making assertions. You can add something like:
expect(input).toHaveAttribute("aria-expanded", "true");right after the
userEvent.click(input)
call.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- .changeset/angry-pillows-accept.md (1 hunks)
- packages/components/autocomplete/tests/autocomplete.test.tsx (1 hunks)
- packages/components/autocomplete/src/use-autocomplete.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/autocomplete/src/use-autocomplete.ts
🔇 Additional comments (2)
packages/components/autocomplete/__tests__/autocomplete.test.tsx (2)
865-886
: LGTM: Well-structured test for auto-highlight functionality.This test case effectively verifies that the first non-disabled item is auto-highlighted when the
isAutoHighlight
prop is set to true. The test structure is clear and follows best practices for component testing.
888-910
: LGTM: Comprehensive test for auto-highlight with disabled items.This test case effectively verifies that the auto-highlight feature skips disabled items. It correctly sets up the component with disabled keys and checks that the first non-disabled item (third in the list) is highlighted.
|
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.
please rename the prop to autoHighlight
.
Prop renamed to autoHiglight |
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
🧹 Outside diff range and nitpick comments (1)
apps/docs/content/docs/components/autocomplete.mdx (1)
87-88
: Improved clarity for Read Only behaviorThe updated description provides a clearer explanation of the Read Only state behavior. This change enhances user understanding of how the Autocomplete component functions when
isReadOnly
is set to true.Consider adding a brief note about why a user might want to use this feature, to provide more context. For example:
If you pass the `isReadOnly` property to the Autocomplete, the Listbox will open to display -all available options, but users won't be able to select any of the listed options. +all available options, but users won't be able to select any of the listed options. This can be useful for displaying a list of options without allowing interaction.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- .changeset/angry-pillows-accept.md (1 hunks)
- apps/docs/content/components/autocomplete/auto-highlight.ts (1 hunks)
- apps/docs/content/components/autocomplete/index.ts (2 hunks)
- apps/docs/content/docs/components/autocomplete.mdx (2 hunks)
- packages/components/autocomplete/tests/autocomplete.test.tsx (1 hunks)
- packages/components/autocomplete/src/use-autocomplete.ts (5 hunks)
- packages/components/autocomplete/stories/autocomplete.stories.tsx (3 hunks)
- packages/components/listbox/src/listbox.tsx (2 hunks)
- packages/components/listbox/src/use-listbox-item.ts (4 hunks)
- packages/components/listbox/src/use-listbox.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- .changeset/angry-pillows-accept.md
- apps/docs/content/components/autocomplete/auto-highlight.ts
- apps/docs/content/components/autocomplete/index.ts
- packages/components/autocomplete/tests/autocomplete.test.tsx
- packages/components/autocomplete/src/use-autocomplete.ts
- packages/components/autocomplete/stories/autocomplete.stories.tsx
- packages/components/listbox/src/listbox.tsx
- packages/components/listbox/src/use-listbox-item.ts
- packages/components/listbox/src/use-listbox.ts
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/autocomplete.mdx
[uncategorized] ~325-~325: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...={autocompleteContent.sections} /> ### Auto Highlight If you pass theautoHighlight
proper...(AUTO_HYPHEN)
[uncategorized] ~327-~327: “to the” seems less likely than “to”.
Context: ...f you pass theautoHighlight
property to the Autocomplete, the Listbox will show the...(AI_HYDRA_LEO_CP_TO_THE_TO)
🔇 Additional comments (1)
apps/docs/content/docs/components/autocomplete.mdx (1)
Line range hint
1-1
: Summary of Autocomplete documentation changesThe updates to the Autocomplete component documentation introduce a new Auto-Highlight feature and improve the clarity of the Read Only behavior. These changes enhance the overall quality of the documentation. However, there are a few key points that need to be addressed:
- Ensure consistency in the naming and hyphenation of the new Auto-Highlight feature throughout the document.
- Verify the correct property name for the Auto-Highlight feature (
isAutoHighlight
orautoHighlight
) and use it consistently.- Expand the description of the Auto-Highlight feature to provide more context and use cases.
- Update the API section to include the new Auto-Highlight property with its description and default value.
Addressing these points will significantly improve the clarity and completeness of the Autocomplete component documentation.
To ensure all instances of the new feature are consistently named and documented, you can run the following command:
This will help identify any inconsistencies in naming or missed occurrences of the new feature throughout the file.
✅ Verification successful
All instances of the
autoHighlight
property inapps/docs/content/docs/components/autocomplete.mdx
are consistently named using camelCase. No variations such asauto-highlight
orauto_highlight
were found, ensuring naming consistency throughout the documentation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
rg -i 'auto.?highlight' apps/docs/content/docs/components/autocomplete.mdx
Length of output: 304
Script:
#!/bin/bash # Search for exact property name `autoHighlight` in the specified file rg -w 'autoHighlight' apps/docs/content/docs/components/autocomplete.mdx # Additionally, search for any hyphenated or snake_case variations to ensure consistency rg -w 'auto-highlight' apps/docs/content/docs/components/autocomplete.mdx rg -w 'auto_highlight' apps/docs/content/docs/components/autocomplete.mdxLength of output: 429
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
The following case failed. Please add a new test case to cover that as well.
- go to autocomplete story
- open autocomplete (either by clicking input or by clicking selection button)
- the listbox is open, and the first item is highlighted (expected)
- close it
- open it again, the first item is not highlighted.
@@ -1061,3 +1081,10 @@ export const FullyControlled = { | |||
...defaultProps, | |||
}, | |||
}; | |||
|
|||
export const WithAutoHighlight = { | |||
render: AutoHighlightTemplate, |
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.
there's no need to create a new template for that. You can simply use Template
one and just pass autoHighlight
to args below.
@@ -92,6 +92,11 @@ interface Props<T> extends Omit<HTMLNextUIProps<"ul">, "children"> { | |||
* The menu items classNames. | |||
*/ | |||
itemClasses?: ListboxItemProps["classNames"]; | |||
/** | |||
* Whether to automatically highlight the first item in the list. |
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.
first non-disabled item
Closes # N/A
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
autoHighlight
, to enhance user experience in both autocomplete and listbox components.Documentation
Autocomplete
component to include new features and clarify existing properties.Bug Fixes