-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat(Select): add selected elements counter on multiple selection #1368
Conversation
Playwright Test Component is ready. |
Preview is ready. |
src/components/Select/components/SelectControl/SelectControl.tsx
Outdated
Show resolved
Hide resolved
@@ -148,4 +157,13 @@ export type SelectClearProps = SelectClearIconProps & { | |||
onMouseLeave: (e: React.MouseEvent) => void; | |||
}; | |||
|
|||
export type SelectCounterProps = { | |||
/** number of selected elements to show */ | |||
count: number; |
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 suppose it could also be a 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.
We use and manage this value only inside our code, so it could be only number in this 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 suppose we use it here https://github.com/gravity-ui/uikit/pull/1368/files#diff-c1510619d5ba5e80032efb9ba67b29e63307d6a80b81fc2cd3edc466a05d0439R57 as a public interface?
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.
Yes we pass this parameter to render props. Ok, let's make it string if it is more convenient)
fb78910
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.
No, I suggest to use type string | number
to have opportunity to set values 1
, 1k
an so on
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.
Ok, remade it) d7ed58c
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.
return number) 00f69d6
Based on DATAUI-2111