Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
azozulya
left a comment
azozulya
left a comment
There was a problem hiding this comment.
У меня замечания по наименованию. Так как мы используем модули, то стили изолированы и можно не пользоваться бэм наименованиями.
Вместо .checkbox__icon можно писать .icon Короткие классы - это удобно.
| }: CustomCheckboxProps) => { | ||
| return ( | ||
| <label | ||
| className={cx(styles.checkbox, labelClassName, isPositionRight && styles['checkbox--right'])} |
There was a problem hiding this comment.
Правильнее будет написать
{ styles['checkbox--right']: isPositionRight }
There was a problem hiding this comment.
Изначально я так писала, но почему-то такое написание вызывает ошибку... Может, где-то ошиблась в синтаксисе, но не могу понять, в чем дело.
There was a problem hiding this comment.
Да, я ошиблась. Надо в скобки взять {[styles['checkbox--right']]: isPositionRight}
| value={value} | ||
| name={name} | ||
| id={id} | ||
| className={cx(styles['visually-hidden'], inputClassName)} |
There was a problem hiding this comment.
styles['visually-hidden'] удобнее было бы записать styles.visuallyHidden Т.е. использовать camelCase в наименовании классов.
| onChange?: (event: ChangeEvent<HTMLInputElement>) => void; | ||
| inputClassName?: string; | ||
| labelClassName?: string; | ||
| labelText?: string; |
There was a problem hiding this comment.
В подписи к чекбоксу могут быть ссылки, например ссылки на политики конфиденциальности.
Я бы использовала children, но с другой стороны такое может не встретиться вообще или встретиться только один раз, тогда для такого случая думаю сойдет передать в строке разметку и преобразовать парсером
|
|
||
| &--right { | ||
| justify-content: space-between; | ||
| } |
There was a problem hiding this comment.
Весь элемент кликабельный, не хватает cursor: pointer
| padding: 0; | ||
|
|
||
| &--right { | ||
| justify-content: space-between; |
There was a problem hiding this comment.
Не совсем поняла, почему при тексте слева или справа элемент себя совсем по-разному ведет. Почему нельзя сделать промежуток между чекбоксом и текстом с помощью gap? Тогда и марджин не нужен. Сейчас элемент растягивается на весь контейнер, чекбокс улетает далеко от текста, если выбрать isPositionRight, и нужно задавать макс ширину дополнительно
There was a problem hiding this comment.
К сожалению, не нашла в макете пример с правым чекбоксом. Исходила из следующих соображений.
-
Если задать gap, когда текст слева, то все иконки будут находиться на разном уровне, т.к. длина текста в разных label не будет одинаковой. Насколько я поняла, текст и иконка должны быть разнесены по разным сторонам контейнера (как на скриншоте разнесены дата и значок календаря). Тут нужно понимать, какой ожидается результат: одинаковое расстояние между текстом и иконкой во всех чекбоксах или разнесение текста и иконки по краям? Возможно, такой вариант чекбокса будет использоваться в мобильной версии.
-
Чекбоксы не будут находиться на странице сами по себе. Они в любом случае будут в каком-то контейнере (например, в форме, как на скрине). Тогда ширина чекбокса с левым текстом и правой иконкой будет зависеть от ширины этого контейнера, т.е. задавать макс. ширину нет смысла и может даже навредить, т.к. мы не знаем, какой длины текст будет находиться в label. Если же такой вариант чекбокса предполагается использовать в мобильной версии, то ширина будет ограничена либо шириной экрана, либо контейнером.
There was a problem hiding this comment.
Выбор даты это совсем другой элемент, но логика ясна.
Стили можно переопределять при вызове этого компонента, но лучше конечно изначально задать оптимальные
There was a problem hiding this comment.
Выбор даты я привела в качестве чисто визуального примера, т.к. не нашла в макете пример с таким чекбоксом. Поэтому вопрос в том, как предполагается использовать чекбокс такого типа. Если он должен выглядеть так, как элемент с выбором даты, то это как раз достигается с помощью space-between. Если он должен выглядеть по-другому (т.е. важно именно расстояние между текстом и иконкой), то использовать gap. Т.е. это разные задачи, которые решаются разными способами. Изменить на gap или оставить, как есть, до появления в макете такого элемента?
| width: 24px; | ||
| height: 24px; | ||
| box-sizing: border-box; | ||
| border: 2px solid $neutral-text-light-color; |
There was a problem hiding this comment.
Ширина обводки в макете соответствует одному пикселю
| height: 24px; | ||
| box-sizing: border-box; | ||
| background-color: $neutral-background-main-color; | ||
| border: 2px solid $neutral-text-light-color; |
There was a problem hiding this comment.
Также один пиксель
| overflow: hidden; | ||
| } | ||
|
|
||
| .radio { |
There was a problem hiding this comment.
Также не хватает cursor: pointer
| overflow: hidden; | ||
| } | ||
|
|
||
| .toggle { |
4 participants
Сверстаны чекбоксы, радиокнопки и переключатели согласно UI-киту