Skip to content

feat(FormItem): tokenized#2600

Merged
SevereCloud merged 2 commits intomasterfrom
SevereCloud/issue2542
Jun 14, 2022
Merged

feat(FormItem): tokenized#2600
SevereCloud merged 2 commits intomasterfrom
SevereCloud/issue2542

Conversation

@SevereCloud
Copy link
Copy Markdown
Contributor

@SevereCloud SevereCloud commented May 24, 2022 •
edited
Loading

Перевод на токены FormItem и FormLayoutGroup

Чеклист перевода компонента на vkui-tokens

  • В стилях компонента не осталось платформенных селекторов(оставлен для обратной совместимости)
  • Если в стилях встречаются токены из Appearance, то их нужно не удалять, а дополнять фоллбэком на соответствующий токен из vkui-tokens
  • В tsx компонента не осталось логики, которая зависит от платформы(оставлено для обратной совместимости)
  • Компонент добавлен в src/tokenized/index.ts (в src/index.ts он так же должен быть)
  • Имя компонента добавлено в массив из styleguide/tokenized.js

Разное

Борьба за 2 пикселя
До После
image image
Выравнивание компонентов
До После
image image

Документация компонентов

@SevereCloud SevereCloud requested a review from a team as a code owner May 24, 2022 11:03
@SevereCloud SevereCloud marked this pull request as draft May 24, 2022 11:04
@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented May 24, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 73b2cb2:

Sandbox Source
VKUI - default example Configuration

@SevereCloud SevereCloud mentioned this pull request May 24, 2022
Closed
89 tasks
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 24, 2022 •
edited
Loading

size-limit report 📦

Path Size
JS 257.93 KB (+0.03% 🔺)
JS (gzip) 76.36 KB (-0.01% 🔽)
JS (brotli) 64.4 KB (+0.01% 🔺)
JS, unstable 29.56 KB (0%)
CSS 271.71 KB (+0.23% 🔺)
CSS (gzip) 38.14 KB (+0.31% 🔺)
CSS (brotli) 30.82 KB (+0.08% 🔺)
CSS, unstable 942 B (0%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 24, 2022

👀 Styleguide deployed

See the styleguide for this PR at https://vkcom.github.io/VKUI/pull/2600/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 24, 2022 •
edited
Loading

Changed screenshots

formitem-android-light-1
formitem-ios-light-1
formitem-vkcom-light-1
formlayoutgroup-android-light-1
formlayoutgroup-ios-light-1
formlayoutgroup-vkcom-light-1

Code coverage

lines3801 / 484578.45%
statements3870 / 493778.38%
functions842 / 105379.96%
branches3232 / 454471.12%
branchesTrue0 / 0100.00%

Generated by 🚫 dangerJS against 73b2cb2

@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 21fbc71 to 58e25ad Compare May 24, 2022 15:56
@SevereCloud SevereCloud marked this pull request as ready for review May 24, 2022 16:07
@SevereCloud SevereCloud requested a review from a team as a code owner May 24, 2022 16:07
@SevereCloud
Copy link
Copy Markdown
Contributor Author

SevereCloud commented May 24, 2022

Дизайн ревью ✅ (by @Fliqle)

@eugpoloz
Copy link
Copy Markdown
Contributor

eugpoloz commented May 24, 2022

А задачи для FormLayoutGroup у нас отдельной нет?

@SevereCloud
Copy link
Copy Markdown
Contributor Author

SevereCloud commented May 24, 2022

А задачи для FormLayoutGroup у нас отдельной нет?

Неа

@eugpoloz
Copy link
Copy Markdown
Contributor

eugpoloz commented May 24, 2022

@SevereCloud а сделай тогда тоже или дополни задачку про FormItem, чтоб было видно, какие компоненты учлись.

eugpoloz
eugpoloz suggested changes May 24, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@eugpoloz eugpoloz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай разделим сущности и оставим сейчас в этом пулле только токенизацию и переезд на именованные экспорты.

Доступность в этом случае надо бы очень внимательно проверить, aria-метки — это часто показатель, что мы делаем что-то неправильно; а токенизация и именованные экспорты нужны как можно быстрее.

@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from c6960d1 to 4e746b8 Compare May 24, 2022 19:19
@SevereCloud SevereCloud requested a review from eugpoloz May 24, 2022 19:20
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 4e746b8 to 4897257 Compare May 25, 2022 12:26
inomdzhon
inomdzhon previously approved these changes May 26, 2022
View reviewed changes
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 4897257 to 2caf474 Compare May 30, 2022 16:08
@SevereCloud SevereCloud removed the request for review from a team May 30, 2022 16:08
inomdzhon
inomdzhon previously approved these changes May 31, 2022
View reviewed changes
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 2caf474 to 26fd611 Compare June 1, 2022 09:30
eugpoloz
eugpoloz reviewed Jun 2, 2022
View reviewed changes
src/components/FormItem/FormItem.css
flex-basis: 0;
padding: 0;
min-width: 0;
min-width: 1px;
Copy link
Copy Markdown
Contributor

@eugpoloz eugpoloz Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А расскажи, почему здесь 0 превратился в 1px, чем 0 не подошел?

Copy link
Copy Markdown
Contributor Author

@SevereCloud SevereCloud Jun 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Copy Markdown
Contributor

@eugpoloz eugpoloz Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SevereCloud все еще не очень поняла, раскрой мысль, пожалуйста. х)

Copy link
Copy Markdown
Contributor Author

@SevereCloud SevereCloud Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2022-06-10.20.18.37.mov

@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 50a5f27 to 6adb411 Compare June 2, 2022 12:10
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from 6adb411 to a22295c Compare June 9, 2022 14:18
inomdzhon
inomdzhon previously approved these changes Jun 9, 2022
View reviewed changes
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from a22295c to ec19a13 Compare June 9, 2022 14:40
inomdzhon
inomdzhon previously approved these changes Jun 9, 2022
View reviewed changes
@SevereCloud SevereCloud force-pushed the SevereCloud/issue2542 branch from dc789bd to 73b2cb2 Compare June 10, 2022 10:15
@eugpoloz eugpoloz self-requested a review June 10, 2022 15:27
eugpoloz
eugpoloz reviewed Jun 10, 2022
View reviewed changes
src/components/Removable/Removable.tsx
{children}

<span vkuiClass="Removable__offset" aria-hidden="true"></span>
<span vkuiClass="Removable__offset" aria-hidden />
Copy link
Copy Markdown
Contributor

@eugpoloz eugpoloz Jun 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я тут подумала и поняла, что вообще этот offset нам нужен только для FormLayoutGroup. Может, туда и перенесем? А тут оба использования выпилим.

Copy link
Copy Markdown
Contributor Author

@SevereCloud SevereCloud Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Он еще в FormItem нужен

@eugpoloz
Copy link
Copy Markdown
Contributor

eugpoloz commented Jun 10, 2022

@SevereCloud короче! Все теперь вполне ок, а на следующей неделе я в отпуске, поэтому лайк тебе авансом, но если ответишь на мои вопросики и комменты — буду признательна. 😆

@SevereCloud SevereCloud merged commit 3364259 into master Jun 14, 2022
@SevereCloud SevereCloud deleted the SevereCloud/issue2542 branch June 14, 2022 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inomdzhon inomdzhon inomdzhon approved these changes

+1 more reviewer

@eugpoloz eugpoloz eugpoloz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature] Токенизировать FormLayoutGroup [Feature] Токенизировать FormItem

3 participants

@SevereCloud @eugpoloz @inomdzhon