feat: add search box and title above model list #5987
Conversation
|
@chenyu-01 is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces enhancements to the Selector component, adding search functionality and improving the styling of UI elements. Changes include the addition of new state variables and functions for managing search input and filtered results in the component logic. The styling updates in the SCSS file create a more structured layout for the selector interface. Localization files are updated to include new entries for search-related text in both Chinese and English, ensuring the feature is accessible to a wider audience. Changes
Sequence DiagramsequenceDiagram
participant User
participant SearchInput
participant Selector
participant ItemList
User->>SearchInput: Type search text
SearchInput->>Selector: Trigger onSearch
Selector->>Selector: Filter items
Selector->>ItemList: Update displayed items
ItemList-->>User: Show filtered results
Poem
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: 0
π§Ή Nitpick comments (3)
app/components/ui-lib.tsx (3)
510-523: Consider adding a debounce or memoization for the search logic.
Repeatedly filtering large lists on every input event may impact performance. A debounce approach or memoizing could enhance efficiency for larger data sets.
527-541: Include accessibility attributes on the search input.
Applying appropriate labels or aria attributes on the search input (e.g.,aria-label="Search"or associating a<label>) can improve accessibility and assist screen readers.
543-582: Leverage a consistent styling or standardized component for the selected indicator.
Changing the inline styles for the selected indicator to a dedicated SCSS class helps ensure visual consistency and maintainability.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
app/components/ui-lib.module.scss(2 hunks)app/components/ui-lib.tsx(1 hunks)app/locales/cn.ts(1 hunks)app/locales/en.ts(1 hunks)
π Additional comments (5)
app/locales/cn.ts (1)
761-763: Localized strings look good.
Thank you for covering the edge case with "沑ζη»ζ" to handle empty search results.
app/locales/en.ts (1)
767-769: Clear and concise localization updates.
This ensures a consistent user experience for English-speaking users when no results match their search.
app/components/ui-lib.module.scss (3)
81-81: Revising the border radius only on the bottom corners.
Aligning the .list with the new .selector-bar top radius is logical, but ensure the change doesnβt conflict with other places where .list styling is reused.
316-341: New search bar styles are clear and well-organized.
Using distinct class names for .selector-bar, .selector-title, and .selector-search-input avoids conflicts with other UI elements.
346-346: Lowering max-height from 90vh to 50vh.
This will constrain the visible area of the list for larger screen sizes. Verify that it doesnβt negatively impact usability for scenarios with extensive item lists.
|
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
|
I found some issues below that you can fix.
|
There was a problem hiding this comment.
Actionable comments posted: 3
π§Ή Nitpick comments (2)
app/components/ui-lib.tsx (2)
510-523: Enhance search functionality and optimize state updatesA few suggestions to improve the search implementation:
- Include subtitle in the search to improve findability
- Remove unnecessary spread operators
- Consider debouncing the search for better performance with large lists
function onSearch(text: string) { setSearchText(text); if (text === "") { - setFilteredItems([...props.items]); + setFilteredItems(props.items); return; } - // filter by items title + // filter by title and subtitle const newItems = props.items.filter((item) => - item.title.toLowerCase().includes(text.toLowerCase()), + item.title.toLowerCase().includes(text.toLowerCase()) || + (item.subTitle?.toLowerCase()?.includes(text.toLowerCase()) ?? false) ); - setFilteredItems([...newItems]); + setFilteredItems(newItems); }
566-577: Move inline styles to CSS moduleExtract the selection indicator styles to the CSS module for better maintainability and consistency with the codebase's styling approach.
+// In ui-lib.module.scss +.selector-item-indicator { + height: 10px; + width: 10px; + background-color: var(--primary); + border-radius: 10px; +} -<div - style={{ - height: 10, - width: 10, - backgroundColor: "var(--primary)", - borderRadius: 10, - }} -></div> +<div className={styles["selector-item-indicator"]} /> -<></> +{null}
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (2)
app/components/ui-lib.module.scss(2 hunks)app/components/ui-lib.tsx(1 hunks)
π Additional comments (1)
app/components/ui-lib.module.scss (1)
81-81: LGTM! Border radius adjustment aligns with new design
The border-radius change correctly implements the visual connection between the search bar and list.
app/components/ui-lib.module.scss
Outdated
| .list { | ||
| max-height: 90vh; | ||
| max-height: 50vh; |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Improve list size handling to prevent jarring changes
The fixed 50vh height might cause abrupt size changes during typing. Consider a more flexible approach:
- max-height: 50vh;
+ min-height: 30vh;
+ max-height: calc(90vh - 120px); /* Account for search bar and padding */π 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.
| max-height: 50vh; | |
| min-height: 30vh; | |
| max-height: calc(90vh - 120px); /* Account for search bar and padding */ |
| <input | ||
| type="text" | ||
| className={styles["selector-search-input"]} | ||
| placeholder={Locale.UI.Search} | ||
| autoFocus | ||
| onInput={(e) => onSearch(e.currentTarget.value)} | ||
| /> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Improve accessibility and event handling of search input
Add accessibility attributes and use onChange for better cross-browser compatibility.
<input
type="text"
className={styles["selector-search-input"]}
placeholder={Locale.UI.Search}
autoFocus
- onInput={(e) => onSearch(e.currentTarget.value)}
+ onChange={(e) => onSearch(e.target.value)}
+ aria-label={Locale.UI.Search}
+ role="searchbox"
/>π 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.
| <input | |
| type="text" | |
| className={styles["selector-search-input"]} | |
| placeholder={Locale.UI.Search} | |
| autoFocus | |
| onInput={(e) => onSearch(e.currentTarget.value)} | |
| /> | |
| <input | |
| type="text" | |
| className={styles["selector-search-input"]} | |
| placeholder={Locale.UI.Search} | |
| autoFocus | |
| onChange={(e) => onSearch(e.target.value)} | |
| aria-label={Locale.UI.Search} | |
| role="searchbox" | |
| /> |
β¦ config selector use same title
|
Try setting a fixed height of select boxο½ |
|
There was a problem hiding this comment.
Actionable comments posted: 2
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
app/components/ui-lib.module.scss(2 hunks)
π Additional comments (1)
app/components/ui-lib.module.scss (1)
81-84: LGTM: Border radius adjustment aligns with new selector bar design
The border-radius modification correctly implements a seamless connection between the list and the new selector bar above it.
| animation: slide-in ease 0.3s; | ||
| width: 30rem; | ||
| height: 60vh; |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Improve responsive handling of selector dimensions
The fixed dimensions might cause layout issues on different screen sizes and during interactions. Consider:
&-content {
animation: slide-in ease 0.3s;
- width: 30rem;
- height: 60vh;
+ width: min(30rem, 90vw); /* Responsive width */
+ max-height: 60vh; /* Use max-height instead of fixed height */
+ min-height: 300px; /* Prevent too small height */
+
+ @media screen and (max-width: 480px) {
+ width: 95vw; /* Fuller width on mobile */
+ max-height: 70vh; /* More vertical space on mobile */
+ }This change will:
- Prevent content overflow on smaller screens
- Reduce size changes during typing
- Improve mobile experience
π 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.
| animation: slide-in ease 0.3s; | |
| width: 30rem; | |
| height: 60vh; | |
| animation: slide-in ease 0.3s; | |
| width: min(30rem, 90vw); /* Responsive width */ | |
| max-height: 60vh; /* Use max-height instead of fixed height */ | |
| min-height: 300px; /* Prevent too small height */ | |
| @media screen and (max-width: 480px) { | |
| width: 95vw; /* Fuller width on mobile */ | |
| max-height: 70vh; /* More vertical space on mobile */ | |
| } |
| .selector-bar { | ||
| background-color: var(--white); | ||
| border: solid var(--border-in-light); | ||
| border-top-left-radius: 10px; | ||
| border-top-right-radius: 10px; | ||
| min-height: 40px; | ||
| width: 100%; | ||
| } | ||
| .selector-title { | ||
| font-size: large; | ||
| font-weight: bold; | ||
| padding: 10px; | ||
| } | ||
| .selector-search-container { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| padding: 10px; | ||
| border-bottom: solid var(--border-in-light); | ||
| } | ||
| .selector-search-input { | ||
| padding: 5px; | ||
| margin-right: 5px; | ||
| flex-grow: 1; | ||
| max-width: none; | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Enhance selector bar styling for better visual consistency
The selector bar implementation needs improvements for better visual consistency and user experience:
- Add consistent spacing and alignment:
.selector-bar {
background-color: var(--white);
border: solid var(--border-in-light);
border-top-left-radius: 10px;
border-top-right-radius: 10px;
min-height: 40px;
width: 100%;
+ display: flex;
+ flex-direction: column;
}
.selector-title {
font-size: large;
font-weight: bold;
padding: 10px;
+ padding-bottom: 5px; /* Reduce spacing before search */
}
.selector-search-container {
display: flex;
align-items: center;
justify-content: space-between;
padding: 10px;
+ padding-top: 5px; /* Balance with title padding */
border-bottom: solid var(--border-in-light);
}
.selector-search-input {
padding: 5px;
margin-right: 5px;
flex-grow: 1;
max-width: none;
+ height: 30px; /* Consistent height */
+ border: 1px solid var(--border-in-light);
+ border-radius: 6px;
+ outline: none;
+
+ &:focus {
+ border-color: var(--primary);
+ }
}π 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.
| .selector-bar { | |
| background-color: var(--white); | |
| border: solid var(--border-in-light); | |
| border-top-left-radius: 10px; | |
| border-top-right-radius: 10px; | |
| min-height: 40px; | |
| width: 100%; | |
| } | |
| .selector-title { | |
| font-size: large; | |
| font-weight: bold; | |
| padding: 10px; | |
| } | |
| .selector-search-container { | |
| display: flex; | |
| align-items: center; | |
| justify-content: space-between; | |
| padding: 10px; | |
| border-bottom: solid var(--border-in-light); | |
| } | |
| .selector-search-input { | |
| padding: 5px; | |
| margin-right: 5px; | |
| flex-grow: 1; | |
| max-width: none; | |
| } | |
| .selector-bar { | |
| background-color: var(--white); | |
| border: solid var(--border-in-light); | |
| border-top-left-radius: 10px; | |
| border-top-right-radius: 10px; | |
| min-height: 40px; | |
| width: 100%; | |
| display: flex; | |
| flex-direction: column; | |
| } | |
| .selector-title { | |
| font-size: large; | |
| font-weight: bold; | |
| padding: 10px; | |
| padding-bottom: 5px; /* Reduce spacing before search */ | |
| } | |
| .selector-search-container { | |
| display: flex; | |
| align-items: center; | |
| justify-content: space-between; | |
| padding: 10px; | |
| padding-top: 5px; /* Balance with title padding */ | |
| border-bottom: solid var(--border-in-light); | |
| } | |
| .selector-search-input { | |
| padding: 5px; | |
| margin-right: 5px; | |
| flex-grow: 1; | |
| max-width: none; | |
| height: 30px; /* Consistent height */ | |
| border: 1px solid var(--border-in-light); | |
| border-radius: 6px; | |
| outline: none; | |
| &:focus { | |
| border-color: var(--primary); | |
| } | |
| } |
π» εζ΄η±»ε | Change Type
π εζ΄θ―΄ζ | Description of Change
Can now search llm models when selecting them
π θ‘₯ε δΏ‘ζ― | Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
Localization