-
Notifications
You must be signed in to change notification settings - Fork 381
RI-7189: Add SelectionBox component #4702
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
base: feature/RI-6855/vector-search
Are you sure you want to change the base?
RI-7189: Add SelectionBox component #4702
Conversation
redisinsight/ui/src/components/new-index/selection-box/SelectionBox.tsx
Outdated
Show resolved
Hide resolved
} | ||
|
||
export const StyledBoxContent = styled.div` | ||
padding: 16px; |
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.
IMO we should strive to use the design system of redis-ui, or make our own, but in any case, avoid hardcoding random values. This goes for color values, for spacings, border radii etc.
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.
I'm definitely in favor of using them, but I couldn't find where they’re defined (or how to use the ones from redis-ui
). I noticed a few components using hardcoded values, which is why I left it as-is for now. If you could clarify or point me in the right direction, that would be helpful.
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.
Since redis-ui uses styled components, all these values are in the theme
used for styled components.
You can obtain the theme via a hook:
import { useTheme } from '@redis-ui/styles'
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.
Also each styled component gets a theme
prop, that is the same as the result of useTheme
See /redisinsight/ui/src/components/base/display/loader/Loader.tsx
for some example usage.
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.
I applied changes regarding this point.
export const StyledDisabledBar = styled.div` | ||
padding: 2px 0; | ||
/* Design color: #f4f5f6 */ | ||
background: var(--comboBoxBadgeBgColor); |
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.
where is this defined?
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.
You can find them in redisinsight/ui/src/styles/themes
, specifically within darkTheme
and lightTheme
.
Not sure if this is the best source to pull from - if you have a better suggestion, feel free to share!
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.
Now I changed it to get the values from the redis-ui
theme.
StyledTitle, | ||
} from './SelectionBox.styles' | ||
|
||
export interface BoxSelectionOption<T extends string = string> |
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.
isn't that already exported above?
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.
Not sure if I understand the question properly, but this is an extension to BoxSelectionGroupBox
as we want to support text
as well. So I'm exporting the type for the consumers of this component.
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.
redisinsight/ui/src/components/new-index/selection-box/index.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: pd-redis <[email protected]>
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.
LGTM
This is a component based on: BoxSelectionGroup with some adjustments on the style.
Preview
Usage
It's intended to be used within
<BoxSelectionGroup />
.Example form: