Fixed: Not display buttons when search bar is empty #1927
Conversation
|
@Nil2000 is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Note: A complete assessment would require reviewing the full implementation of the Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
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: 1
🧹 Outside diff range and nitpick comments (1)
packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx (1)
33-36: Consider allowing multi-word searchWhile clearing results for empty input is correct, treating any space as an empty input might be too restrictive. Users might want to search for phrases containing spaces.
Consider this alternative implementation:
-if (inputValue === "" || inputValue.includes(" ")) { +if (inputValue.trim() === "") { setFilteredItems([]); return; }Also applies to: 40-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx(3 hunks)packages/embeds/js/src/features/blocks/inputs/buttons/components/MultipleChoicesForm.tsx(3 hunks)
🔇 Additional comments (2)
packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx (2)
56-56: LGTM: Consistent clear behavior
Setting filteredItems to an empty array on clear is consistent with the PR objective and the new initialization behavior.
18-20: LGTM: Improved initial state handling
The conditional initialization of filteredItems based on isSearchable aligns with the PR objective. When search is enabled, starting with an empty array prevents displaying buttons before user input.
Let's verify this doesn't break any existing functionality:
✅ Verification successful
Change is safe and works as intended
The conditional initialization of filteredItems is correctly implemented and aligns with the component's behavior:
- When
isSearchableis true, it starts with an empty array and populates items only when the user types (viafilterItemsfunction) - When
isSearchableis false, it shows alldefaultItemsimmediately - The
SearchInputcomponent is only rendered whenisSearchableis true - The filtering logic in
filterItemsfunction works with this initial state
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests or stories that might be affected by this change
rg -l "isSearchable.*true" --type ts --type tsx
Length of output: 77
Script:
#!/bin/bash
# Search for tests or stories that might be affected by this change, using ripgrep without file type filters
rg -l "isSearchable.*true"
# Also search for any usage of the Buttons component to understand the impact
rg -l "Buttons.*isSearchable"
Length of output: 60
Script:
#!/bin/bash
# Let's look for any tests related to the Buttons component
fd "Buttons.*test" --type f
# Search for any stories or usage of the Buttons component
fd "Buttons.*stories" --type f
# Look for the actual implementation to understand the component better
fd "Buttons.tsx" --type f
Length of output: 416
Script:
#!/bin/bash
# Let's examine the content of the Buttons component to understand its usage
cat packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx
# Also check for any imports of this component
rg -l "from.*buttons.*components.*Buttons"
Length of output: 3409
| const [filteredItems, setFilteredItems] = createSignal( | ||
| props.options?.isSearchable ? [] : props.defaultItems, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting shared search logic
The search-related logic is duplicated between Buttons.tsx and MultipleChoicesForm.tsx. Consider extracting this into a shared hook or utility function.
Example implementation:
// useSearchableItems.ts
export const useSearchableItems = <T extends { content?: string }>(
defaultItems: T[],
isSearchable?: boolean
) => {
const [filteredItems, setFilteredItems] = createSignal(
isSearchable ? [] : defaultItems
);
const filterItems = (inputValue: string) => {
if (inputValue.trim() === "") {
setFilteredItems([]);
return;
}
setFilteredItems(
defaultItems.filter((item) =>
item.content?.toLowerCase().includes(inputValue.toLowerCase())
)
);
};
const clearItems = () => setFilteredItems([]);
return { filteredItems, filterItems, clearItems };
};This would:
- Reduce code duplication
- Ensure consistent behavior across components
- Make future modifications easier
Would you like me to create an issue to track this refactoring task?
Also applies to: 55-59, 78-78
|
@baptisteArno Do you mean to have a radio button setting to enable/disable this? |
Exactly, yes! Does that make sense? |
|
@baptisteArno Got it Will work on the issue this away then |
Fixes: #699