Skip to content
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

Улучшения и исправления для PureCell #285

Closed
14 tasks done
NatalyaZ opened this issue Sep 27, 2022 · 3 comments
Closed
14 tasks done

Улучшения и исправления для PureCell #285

NatalyaZ opened this issue Sep 27, 2022 · 3 comments

Comments

@NatalyaZ
Copy link
Contributor

NatalyaZ commented Sep 27, 2022

Проблема

При использовании PureCell в веб-версии приложения были найдены некоторые недочеты. Предлагаю их исправить

PureCell.AmountTitle

  • ошибка: обязательно надо указывать оба параметра minorUnits: 100, minority: 100. Исправить: сделать оба параметра необязательными
  • ошибка: в параметр amount попадают все свойства Amount, в том числе и класс. Исправить: объединить тип компонента с AmountProps
  • требование от клика: для минорной части(в целом для суммы в заголовке) надо --amount-minor-part-font-weight: 500 (https://www.figma.com/file/OiFlIdSpmFYZ5zsFGWA3TD/Web-%3A%3A-Alfa-Online-Components?node-id=43968%3A229857). Исправить: исправить css переменную в самом компоненте или добавить пропсу. Ждет подтверждения от дизайнера

PureCell.Addon

  • дополнение: добавились значения для выравнивания по вертикали 'top' | 'center' | 'bottom'. Исправить: добавить значения
  • ошибка: не хватает выравнивания текста по правому краю. Исправить: добавить стили выравнивания текста

PureCell.Text

  • ошибка: у части title есть отступ справа, даже если не использовать value (из-за него выравнивание при direction='vertical' не четко по центру), сохраняется span для value. Исправить: если не указан value, не добавлять отступ и не выводить теги
  • улучшение: при добавлении свойства word-break: break-all; текста отображается больше. Исправление: добавить свойство для обрезания текста в одну строчку

PureCell.Category

  • дополнение: добавилась иконка чека (опционально withReceipt). Исправление: добавить опцию, другое вариант -- сделать отдельный компонент. Ждет подтверждения дизайнера
  • улучшение: при добавлении свойства word-break: break-all; текста отображается больше. Исправление: добавить свойство для обрезания текста в одну строчку

PureCell.Amount

  • вопрос от клика: можно ли включить по дефолту view: 'withZeroMinorPart', transparentMinor: true. Решение: кажется, что никак. Не стоит указывать дефолтные значения для опциональных параметров. Но может появится идея?
  • ошибка: обязательно надо указывать оба параметра minorUnits: 100, minority: 100. Исправить: сделать оба параметра необязательными
  • ошибка: в параметр amount попадают все свойства Amount, в том числе и класс. Исправить: объединить тип компонента с AmountProps, параметр view переименовать в textView

PureCell.Comment -> Comment

  • ошибка: нельзя передать строку в children. Исправление: исправить тип
  • дополнение: добавилось опциональное ограничение по строкам rowLimit?: 2 | 5; Исправление: добавить опцию, по-умолчанию ставить 2
@SiebenSieben
Copy link
Contributor

SiebenSieben commented Sep 28, 2022

  1. PureCell.Category: добавление withReceipt будет слишком заточено под АО. Если потребуются другие иконки, то придётся плодить пропсы. Предлагаю добавить слот rightAddons, а если хотите зашить иконку на уровне компонента, то сделать обёртку в композитах.

  2. PureCell.AmountTitle: уже заточен под АО (соответствующий Typography.Title). А ещё в него зашит view=withZeroMinorPart. Можно сделать его более универсальным и темизируемым через css-переменные, или забить и просто пофиксить (добавить bold=none и transparentMinor=true, тогда он будет наследовать жирность Typography.Title, которая уже 500).

  3. Заметил, что у PureCell.Amount пропса weight, а у обычного Amount — bold. Не уверен, что нужно фиксить, но немного смущает.

@reme3d2y
Copy link
Contributor

PureCell.AmountTitle

ошибка: обязательно надо указывать оба параметра minorUnits: 100, minority: 100. Исправить: сделать оба параметра необязательными

Думаю можем сделать дефолт 100

ошибка: в параметр amount попадают все свойства Amount, в том числе и класс. Исправить: объединить тип компонента с AmountProps

Имеешь ввиду, что класснейм сейчас внутри пропса amount: AmountType ?

требование от клика: для минорной части(в целом для суммы в заголовке) надо --amount-minor-part-font-weight: 500 (https://www.figma.com/file/OiFlIdSpmFYZ5zsFGWA3TD/Web-%3A%3A-Alfa-Online-Components?node-id=43968%3A229857). Исправить: исправить css переменную в самом компоненте или добавить пропсу. Ждет подтверждения от дизайнера

Сейчас не хватает пропсы bold, как у Amount. Добавим

@NatalyaZ
Copy link
Contributor Author

NatalyaZ commented Sep 29, 2022

Имеешь ввиду, что класснейм сейчас внутри пропса amount: AmountType ?

Да. Ну и все остальные свойства

Сейчас не хватает пропсы bold, как у Amount. Добавим

Если что, занимаюсь исправлениями по задаче

NatalyaZ pushed a commit that referenced this issue Oct 4, 2022
NatalyaZ pushed a commit that referenced this issue Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants