Feat: Input refactoring #2592
Feat: Input refactoring #2592
Conversation
Jest Unit tests resultsDuration: 58.552 seconds
Outcome: Passed | Total Tests: 738 | Passed: 737 | Failed: 0
|
src/renderer/features/contacts/ContactFilter/model/contact-filter.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # src/renderer/widgets/CreateWallet/ui/MultisigWallet/components/Signatory.tsx
There was a problem hiding this comment.
Move this test inside storybook play function
| @@ -42,9 +42,16 @@ Disabled.args = { | |||
| disabled: true, | |||
| }; | |||
|
|
|||
| export const Prefix = Template.bind({}); | |||
There was a problem hiding this comment.
Use latest story format, please. You can see examples in the other ui-kit components
| suffixElement, | ||
| onChange, | ||
| onPaste, | ||
| ...props |
| @@ -1,4 +1,4 @@ | |||
| type BaseHTMLInputProps = 'value' | 'required' | 'disabled' | 'placeholder' | 'name' | 'className' | 'autoFocus'; | |||
| type BaseHTMLInputProps = 'value' | 'required' | 'disabled' | 'placeholder' | 'name' | 'autoFocus'; | |||
There was a problem hiding this comment.
Do we really need to move this outside of input props? Overall reading complexity outweighs benefits of shared configuration IMO
| useLayoutEffect(() => { | ||
| if (!prefixElement || !prefixRef.current) return; | ||
|
|
||
| setPaddingLeft(EXTENDED_HORIZONTAL_PADDING + prefixRef.current.getBoundingClientRect().width); |
There was a problem hiding this comment.
you can set ref to top level div element (also, it should be label) and remove all this complex code, made only to support combobox specific api.
Second option - keep ref for input element, but wrap Input into additional div inside Combobox
| const LONG_TEXT = | ||
| 'Lorem ipsum dolor sit amet, consectetur adipisicing elit. Culpa doloribus iusto possimus praesentium ratione temporibus. Aperiam autem cumque esse eum fugit laborum quas! Architecto at, cupiditate dignissimos eveniet sunt voluptatibus.'; | ||
|
|
||
| export const Primary = Template.bind({}); |
There was a problem hiding this comment.
Move this test into storybook play function
| import { cnTw } from '@/shared/lib/utils'; | ||
| import { type HTMLTextAreaProps } from '../common/types'; | ||
|
|
||
| interface Props extends Pick<ComponentPropsWithoutRef<'textarea'>, HTMLTextAreaProps> { |
|
|
||
| interface Props extends Pick<ComponentPropsWithoutRef<'textarea'>, HTMLTextAreaProps> { | ||
| invalid?: boolean; | ||
| onChange?: (value: string) => void; |
There was a problem hiding this comment.
Why onChange callback is optional? Component should be fully controllable IMO
| const DEFAULT_LEFT_PADDING = 11; | ||
| const EXTENDED_LEFT_PADDING = 19; | ||
|
|
||
| type InputProps = { |
There was a problem hiding this comment.
Only name | label | value | onChange are used, maybe you should simplify this component?
Inputcontentui-kitcloses #2587
Before:
After:
