Skip to content

feat(Group): Добавляем подкомпоненты#7395

Merged
andrey-medvedev-vk merged 6 commits intomasterfrom
mendrew/5758/Group/add-submodules
Aug 29, 2024
Merged

feat(Group): Добавляем подкомпоненты#7395
andrey-medvedev-vk merged 6 commits intomasterfrom
mendrew/5758/Group/add-submodules

Conversation

@mendrew
Copy link
Copy Markdown
Contributor

@mendrew mendrew commented Aug 15, 2024


  • Документация фичи

Описание

Разбил Group на подкомпоненты.

<Group.Container>
  <Group.Header>
    <Header>Адреса</Header>
  </Group.Header>
  <CellButton onClick={noop}>Добавить домашний адрес</CellButton>
  <CellButton onClick={noop}>Добавить рабочий адрес</CellButton>
  <Group.Description>
    Для использования в мини-приложениях, Delivery Club, VK Taxi и других сервисах ВКонтакте.
    Эти адреса видны только Вам.
  </Group.Description>
</Group.Container>

Единственное, что в подкомпоненты не вынес это сепаратор между группами.
Вложил его логику в Group.Container, так как Group.Container принимает все основные свойства Group.

Почему не вынес?
До сих пор не уверен в решении.
С одной стороны хорошо бы его вынести, чтобы при использовании подкомпонентного подхода его приходилось бы явно добавлять. И скорее всего его бы не добавляли бы, забывали бы. К тому же он очень гибкий и не всем будет ясна логика такого явного подкомпонента, который то есть, то нет.
Плюс, его бы пришлось явно складывать после группы, так как он управляет расстоянием между группами, это довольно странно и тогда бы было бы не установить связь между группой и сепаратором через контекст.

Неявно его оставлять в Group.Container тоже так себе идея. Он будет продолжать неявно влиять на логику расстояний между группами, как и сейчас, конечно.
Но как-то проще в доке про него рассказать, что он неявно есть, чем явно его добавлять и как-то синкать с группой до него.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 15, 2024

size-limit report 📦

Path Size
JS 378.87 KB (+0.15% 🔺)
JS (gzip) 114.9 KB (+0.1% 🔺)
JS (brotli) 94.35 KB (+0.17% 🔺)
JS import Div (tree shaking) 1.45 KB (0%)
CSS 306.97 KB (0%)
CSS (gzip) 39.37 KB (0%)
CSS (brotli) 31.58 KB (0%)

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Aug 15, 2024

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 15, 2024

e2e tests

Playwright Report

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 15, 2024

👀 Docs deployed

Commit 8c53f92

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 97.36842% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.47%. Comparing base (6aa75b6) to head (8c53f92).
Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
packages/vkui/src/components/Group/GroupHeader.tsx 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7395      +/-   ##
==========================================
- Coverage   94.47%   94.47%   -0.01%     
==========================================
  Files         375      378       +3     
  Lines       11123    11150      +27     
  Branches     3653     3653              
==========================================
+ Hits        10509    10534      +25     
- Misses        614      616       +2     
Flag Coverage Δ
unittests 94.47% <97.36%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mendrew mendrew marked this pull request as ready for review August 15, 2024 15:41
@mendrew mendrew requested a review from a team as a code owner August 15, 2024 15:41
Copy link
Copy Markdown
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

🎉 👏 🙏

inomdzhon
inomdzhon previously approved these changes Aug 20, 2024
mendrew and others added 6 commits August 23, 2024 13:30
Separator is not a subcomponent as it's defined outside of the group
and could be esasily forgotten when subcomponent approach is used.
Otherwise it shows Group.Container as title in the doc
@mendrew mendrew force-pushed the mendrew/5758/Group/add-submodules branch from 6e4033b to 8c53f92 Compare August 23, 2024 10:33
@andrey-medvedev-vk andrey-medvedev-vk merged commit 35f7c57 into master Aug 29, 2024
@andrey-medvedev-vk andrey-medvedev-vk deleted the mendrew/5758/Group/add-submodules branch August 29, 2024 14:15
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

Successfully merging this pull request may close these issues.

[Feature]: Декомпозировать Group

5 participants