feat(new-hope): add react hook form support in combobox #1552
Conversation
|
Theme Builder app deployed! https://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-1552/ |
|
Documentation preview deployed! website:https://plasma.sberdevices.ru/pr/pr-1552/ |
iljs
requested review from
Yeti-or,
neretin-trike,
Yakutoc,
TitanKuzmich and
shuga2704
as code owners
github-actions
bot
requested review from
shuga2704 and
TitanKuzmich
and removed request for
Yeti-or,
Yakutoc,
neretin-trike,
TitanKuzmich and
shuga2704
packages/plasma-new-hope/src/components/Combobox/ComboboxNew/Combobox.tsx
Outdated
Show resolved
Hide resolved
| outerOnChange(newValue as string & string[] & ChangeEvent<HTMLSelectElement>); | ||
| } | ||
|
|
||
| if (!name && multiple && Array.isArray(newValue)) { | ||
| outerOnChange(newValue as string & string[] & ChangeEvent<HTMLSelectElement>); | ||
| } | ||
|
|
||
| if (name && typeof newValue === 'object' && !Array.isArray(newValue)) { | ||
| outerOnChange(newValue as string & string[] & ChangeEvent<HTMLSelectElement>); |
There was a problem hiding this comment.
Для чего эти проверки, если каждый раз кастуем к одному и тому же типу
There was a problem hiding this comment.
Проверки нужны, чтобы отсылать нужный тип. То есть туда может прийти как вариант от формы, так и дефолтное поведение. Тут проверяем, что именно пришло и отсылаем нужный тип
There was a problem hiding this comment.
Кажется, что условия можно объединить, чтобы не дублировать outerOnChange(newValue as string & string[] & ChangeEvent<HTMLSelectElement>);
| @@ -343,7 +383,7 @@ export const comboboxRoot = (Root: RootProps<HTMLInputElement, Omit<ComboboxProp | |||
| listWidth={listWidth} | |||
| target={(referenceRef) => ( | |||
| <StyledTextField | |||
| ref={inputForkRef} | |||
| ref={name ? inputRef : (inputForkRef as React.ForwardedRef<HTMLInputElement>)} | |||
There was a problem hiding this comment.
Можешь вкратце описать, почему итоговая ref зависит от name?
There was a problem hiding this comment.
От name зависит итоговый исходящий тип. Если он есть, то он считает, что это форма. Если нет, то обычный тип.
А ref надо обязательно прокидывать именно в select, если вариант с формой
packages/plasma-new-hope/src/components/Combobox/ComboboxNew/Combobox.types.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Кажется, что нейминг немного нужно поменять. Это же не отдельный компонент Form, а только SelectHidden
There was a problem hiding this comment.
Тут больше логика, что это для формы штука. И потом в select и autocomplite будет нейминг такой же с таким де принципом работы. Поэтому сделал так
packages/plasma-new-hope/src/components/Combobox/ComboboxNew/ui/Form/Form.tsx
Outdated
Show resolved
Hide resolved
| }; | ||
|
|
||
| export const Form = forwardRef<HTMLSelectElement, Props>(({ name, multiple, value, onChange }, ref) => { | ||
| const values = (multiple ? value : [value]) as string[]; |
There was a problem hiding this comment.
Можем избавиться от каста к string[]? Кажется на уровне Props проще переопределить
There was a problem hiding this comment.
Ну можно уйти, если сделать разветвление на multi и на single. Поэтому так проще и практичней
packages/plasma-new-hope/src/components/Combobox/ComboboxNew/utils/syntheticEvent.ts
Outdated
Show resolved
Hide resolved
packages/plasma-new-hope/src/examples/plasma_b2c/components/Combobox/Combobox.stories.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Размножь на другие доки тоже
shuga2704
force-pushed
the
combobox-rhf
branch
from
af2c988
to
e4e21b9
Compare
Адаптация Combobox для ReactHookForm
react-hook-formдля компонентаcomboboxWhat/why changed (Это обязательный заголовок)
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via: