SearchBar & OverflowStack #981
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new UI components (SearchBar and OverflowStack) with comprehensive documentation, updates the SearchBar component styling to use CSS variables, and fixes a text utility variable name.
- Adds SearchBar component with search-on-type, external triggering, and debouncing capabilities
- Adds OverflowStack component for managing item overflow with popover support
- Updates SearchBar CSS to use modern CSS custom properties instead of SCSS variables
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/pages/components/utilities/TextUtilities.tsx | Corrects CSS variable name from --text-size-x-small to --text-size-xx-small |
| examples/pages/components/SearchBar.tsx | Adds comprehensive documentation page for SearchBar component with usage examples |
| examples/pages/components/OverflowStack.tsx | Adds comprehensive documentation page for OverflowStack component with usage examples |
| examples/pages/components/Index.tsx | Registers the new SearchBar and OverflowStack documentation pages |
| app/styles/components/_sd-searchbar.scss | Migrates from SCSS variables to CSS custom properties and removes empty lines |
| app/styles/components/_overflow-stack.scss | Adds new stylesheet for OverflowStack component |
| app/styles/app.scss | Imports the new overflow-stack stylesheet |
| app/styles/_reboot.scss | Minor formatting adjustments for comments |
| app-typescript/index.ts | Exports the new OverflowStack component |
| app-typescript/components/SearchBar.tsx | Enhances SearchBar with refs, debouncing, external methods, and searchOnType feature |
| app-typescript/components/OverflowStack.tsx | Adds new OverflowStack component implementation with auto and fixed overflow modes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| componentDidUpdate(prevProps: IProps) { | ||
| if (prevProps.value !== this.props.value) { | ||
| this.setState({inputValue: this.props.value}); | ||
| this.setState({inputValue: this.props.value || ''}); |
There was a problem hiding this comment.
Component state update uses potentially inconsistent value.
…rdesk-ui-framework into SearchBox-and-OverflowStack
|
hi @fritzSF could you in the future make smaller PRs that are focused on one component/feature area? It's a bit hard to review a bunch of things with a lot of line changes. Reviews will be of better quality if we do that I think Edit: if the feature itself is single and too big, we can split it in parts. I didn't cover that case with my comment |
|
@eos87 could you take a look also. I need to spend more time thinking this through but I don't think we need the |
|
@eos87 i made some changes to the SearchBar also. Generally now input state is handled internally, but I'm not sure yet if the following approach is better: We keep input value storage outside the component and then get rid of all the exposed functions like |
2 participants
Changes on the SerchBar component
New OverflowStack component
A flexible container that stacks any child elements. It can display a fixed number of items and move the remainder into a popover. Optionally, it can automatically adjust which items are visible based on available space, hiding the excess in the popover.
Features: