feat(Button, Link): add new prop - component #3421
Conversation
* Reworked disabled styles for component=a * Added support for icon color customization in component= a
There was a problem hiding this comment.
Типы таких пропов как 'onClick' остались прихардкожеными к старым корневым html-элеменами. Нужно это тоже поправить.
И как идея, я бы еще предложил попробовать ограничить тип пропа component тремя возможными кейсами: button, a и ComponentType. Это облегчит пользователю понимание как пользоваться пропом, убрав всякие сомнительные опции, типа div и все остальные. Плюс сузит зону ответсвенности Button и Link.
| &:hover:enabled svg, | ||
| &.${globalClasses.link}:hover svg { | ||
| color: ${t.btnIconHoverColor}; | ||
| } | ||
| &:disabled svg { | ||
| &:disabled svg, | ||
| &.${globalClasses.link}.${globalClasses.disabled} svg { | ||
| color: ${t.btnIconDisabledColor}; | ||
| } | ||
| &:enabled svg { | ||
| &:enabled svg, | ||
| &.${globalClasses.link} svg { |
There was a problem hiding this comment.
В проп component теперь может приходить не только ссылка, но и любой компонент. В этом случае все цвета тоже должны корректно работать.
| @@ -167,6 +168,9 @@ export interface ButtonProps | |||
| * Стиль кнопки. | |||
| * | |||
| * **Допустимые значения**: `"default"`, `"primary"`, `"success"`, `"danger"`, `"pay"`, `"link"`, `"text"`, `"backless"`. | |||
| * | |||
| * **Вариант `link` устарел.** | |||
| * Если нужна кнопка, выглядящая как ссылка, используйте `Link component=button`. | |||
There was a problem hiding this comment.
Нужно также добавить ворнинг в консоль для тех, кто его сейчас использует.
| // By default the type attribute is 'submit'. IE8 will fire a click event | ||
| // on this button if somewhere on the page user presses Enter while some | ||
| // input is focused. So we set type to 'button' by default. | ||
| type: BUTTON_DEFAULT_ELEMENT, |
There was a problem hiding this comment.
Пропы type и component у Button совсем разные по смыслу. Они чисто случайно пересекаются одним общим возможным значением button, остальные все отличаются. Не стоит их семантически связывать, используя общую константу.
| const sizeClass = this.getSizeClassName(); | ||
|
|
||
| const Root = component as React.ElementType; |
Проблема
В
Buttonнет возможности внутри отрендерить с соответствующими пропами: href, rel и т.д.В
Linkнет публичной возможности отрендерить или из react-router-dom.Была попытка решить в Clickable. Получилось, но череcчур радикально. Нужно переделать на основе существующих компонентов Button и Link, добавив в них проп
component, и настроить типизацию, как вClickable.Решение
Clickable.componentвButtonиLink.Linkи отрисовку состояний.Песочница для теста.
Ссылки
Close IF-1733
Чек-лист перед запросом ревью
Добавлены тесты на все изменения
✅ unit-тесты для логики
✅ скриншоты для верстки и кросс-браузерности
⬜ нерелевантно
Добавлена (обновлена) документация
✅ styleguidist для пропов и примеров использования компонентов
⬜ jsdoc для утилит и хелперов
✅ комментарии для неочевидных мест в коде
⬜ прочие инструкции (
README.md,contributing.mdи др.)⬜ нерелевантно
Изменения корректно типизированы
✅ без использования
any(см. PR2856)⬜ нерелевантно
Прочее
✅ все тесты и линтеры на CI проходят
⬜ в коде нет лишних изменений
✅ заголовок PR кратко и доступно отражает суть изменений (он попадет в changelog)