fix(autocomplete): show popover when emptyContent is provided with allowsCustomValue#5951
Conversation
… and allowsCustomValue
🦋 Changeset detectedLatest commit: 65350f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
@hasegawa-101 is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR fixes a bug where the autocomplete popover failed to display when Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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). (5)
🔇 Additional comments (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/components/autocomplete/src/use-autocomplete.ts (1)
298-305: Empty-content behavior now matches the “custom emptyContent with allowsCustomValue” requirement
hideEmptyContent: allowsCustomValue && !listboxProps?.emptyContentdoes what you want here:
allowsCustomValue=false→ unchanged: empty content can show.allowsCustomValue=true+ customlistboxProps.emptyContent→hideEmptyContentdefaults tofalse, so the popover can open and display that custom content.allowsCustomValue=true+ no customemptyContent→hideEmptyContentstaystrue, and the second new test locks in that behavior.Because
mergePropslater lets consumers still overridehideEmptyContentexplicitly, this is flexible and backwards‑compatible. The only small nit is the naming collision between the destructuredlistboxPropshere andlistBoxPropsfromuseComboBox, which can be confusing when scanning the file; consider renaming one of them in a future cleanup for clarity.If the original issue is meant to also show the default
"No results found."message whenallowsCustomValueis true (not only customemptyContent), this implementation intentionally does not do that; confirm that the current behavior is indeed what you want to ship for #5745.packages/components/autocomplete/stories/autocomplete.stories.tsx (1)
1202-1216: Story accurately exercises the fixed allowsCustomValue + custom emptyContent behaviorThis story is a good, minimal repro:
allowsCustomValue+defaultItems={[]}+listboxProps.emptyContentdirectly covers the bug scenario and validates the new behavior visually.Only minor nit: the child render prop uses
(item: any). SincedefaultItemsis empty here, you could either:
- Drop the render prop entirely and use static children, or
- Give
Autocompletean explicit generic / item type instead ofany.Not required for correctness, but it would keep Storybook examples a bit more type‑safe.
packages/components/autocomplete/__tests__/autocomplete.test.tsx (1)
6-7: Tests correctly pin the new allowsCustomValue + emptyContent behaviorThe new suite does a good job of:
- Verifying that custom
listboxProps.emptyContentis rendered whenallowsCustomValueis true anddefaultItemsis empty.- Explicitly asserting that, without a custom
emptyContent, no listbox (and therefore no default empty content) appears whenallowsCustomValueis true.This matches the updated
hideEmptyContentlogic and guards against regressions.If you want to harden the second test against potential future async changes in how the popover mounts, you could wrap the negative assertion in a
waitFor, e.g.:await waitFor(() => { expect(wrapper.queryByRole("listbox")).toBeNull(); });Not strictly necessary with the current implementation, but it would make the intent resilient to timing changes.
Also applies to: 1093-1151
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/autocomplete/__tests__/autocomplete.test.tsx(2 hunks)packages/components/autocomplete/src/use-autocomplete.ts(1 hunks)packages/components/autocomplete/stories/autocomplete.stories.tsx(1 hunks)
⏰ 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). (5)
- GitHub Check: Continuous Release
- GitHub Check: Tests
- GitHub Check: TypeScript
- GitHub Check: ESLint
- GitHub Check: Build
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-aria-overlay
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-form-reset
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/use-viewport-size
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
2 participants
Closes #5745
📝 Description
This PR fixes an issue where the Autocomplete popover would not appear when
allowsCustomValueis true and the input value does not match any items, even if a customemptyContentwas provided.⛳️ Current behavior (updates)
Currently, when
allowsCustomValueis set to true, thehideEmptyContentprop of the Listbox is forced to true. This causes the popover to remain hidden when there are no search results, regardless of whether the user has provided a customemptyContent🚀 New behavior
The logic in
use-autocomplete.tshas been updated to only default hideEmptyContent to true if allowsCustomValue is true AND no customemptyContentis provided.If a user provides a custom
emptyContent, the popover will now appear to display that content, even whenallowsCustomValueis enabled.💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.